This is an automated email from the ASF dual-hosted git repository. jinwoo pushed a commit to branch support/2.0 in repository https://gitbox.apache.org/repos/asf/geode.git
commit 2819823b0782634cf8543a2f7f91328d00a9e442 Author: Jinwoo Hwang <[email protected]> AuthorDate: Wed Dec 3 11:32:49 2025 -0500 GEODE-10521: Eliminate reflection-based access to java.nio.Buffer internals (#7956) Replace reflection-based access to DirectByteBuffer private APIs with Unsafe field offset access, eliminating the need for --add-opens=java.base/java.nio=ALL-UNNAMED JVM flag. Key Changes: - Enhanced Unsafe wrapper with buffer field access methods * Added cached field offsets (BUFFER_ADDRESS_FIELD_OFFSET, BUFFER_CAPACITY_FIELD_OFFSET) * Added getBufferAddress/setBufferAddress methods * Added getBufferCapacity/setBufferCapacity methods * Field offset access does NOT require --add-opens flags - Refactored AddressableMemoryManager to eliminate reflection * Removed all reflection imports (Constructor, Method, InvocationTargetException) * Removed static volatile reflection caching fields * Reimplemented getDirectByteBufferAddress() using Unsafe.getBufferAddress() * Reimplemented createDirectByteBuffer() using field manipulation * Maintains zero-copy semantics by modifying buffer fields - Removed JAVA_NIO_OPEN flag from MemberJvmOptions * Deleted JAVA_NIO_OPEN constant and documentation * Removed flag from JAVA_11_OPTIONS list * Reduced required JVM flags from 5 to 4 Benefits: - Eliminates security audit findings for --add-opens usage - Improves Java module system compliance - Compatible with Java 17+ strong encapsulation (JEP 403) - Forward compatible with Java 21 - Simplifies deployment configuration - Better performance through cached field offsets - Enables GraalVM native image compilation This change is part of the broader initiative to eliminate all --add-opens and --add-exports flags from Apache Geode for full Java module system compliance. --- .../internal/offheap/AddressableMemoryManager.java | 134 +++++++-------------- .../internal/cli/commands/MemberJvmOptions.java | 8 +- .../geode/unsafe/internal/sun/misc/Unsafe.java | 82 +++++++++++++ 3 files changed, 125 insertions(+), 99 deletions(-) diff --git a/geode-core/src/main/java/org/apache/geode/internal/offheap/AddressableMemoryManager.java b/geode-core/src/main/java/org/apache/geode/internal/offheap/AddressableMemoryManager.java index 7429c97878..473beebf8e 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/offheap/AddressableMemoryManager.java +++ b/geode-core/src/main/java/org/apache/geode/internal/offheap/AddressableMemoryManager.java @@ -14,13 +14,9 @@ */ package org.apache.geode.internal.offheap; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.nio.ByteBuffer; import org.apache.geode.annotations.Immutable; -import org.apache.geode.annotations.internal.MakeNotStatic; import org.apache.geode.internal.JvmSizeUtils; import org.apache.geode.unsafe.internal.sun.misc.Unsafe; @@ -174,114 +170,68 @@ public class AddressableMemoryManager { unsafe.setMemory(addr, size, fill); } - @SuppressWarnings("rawtypes") - @MakeNotStatic - private static volatile Class dbbClass = null; - @SuppressWarnings("rawtypes") - @MakeNotStatic - private static volatile Constructor dbbCtor = null; - @MakeNotStatic - private static volatile boolean dbbCreateFailed = false; - @MakeNotStatic - private static volatile Method dbbAddressMethod = null; - @MakeNotStatic - private static volatile boolean dbbAddressFailed = false; - /** - * Returns the address of the Unsafe memory for the first byte of a direct ByteBuffer. If the - * buffer is not direct or the address can not be obtained return 0. + * Returns the address of the Unsafe memory for the first byte of a direct ByteBuffer. + * + * This implementation uses Unsafe to access the ByteBuffer's 'address' field directly, + * which eliminates the need for reflection with setAccessible() and therefore does not + * require the --add-opens=java.base/java.nio=ALL-UNNAMED JVM flag. + * + * If the buffer is not direct or the address cannot be obtained, returns 0. + * + * @param bb the ByteBuffer to get the address from + * @return the native memory address, or 0 if not available */ - @SuppressWarnings({"rawtypes", "unchecked"}) public static long getDirectByteBufferAddress(ByteBuffer bb) { if (!bb.isDirect()) { return 0L; } - if (dbbAddressFailed) { + if (unsafe == null) { return 0L; } - Method m = dbbAddressMethod; - if (m == null) { - Class c = dbbClass; - if (c == null) { - try { - c = Class.forName("java.nio.DirectByteBuffer"); - } catch (ClassNotFoundException e) { - // throw new IllegalStateException("Could not find java.nio.DirectByteBuffer", e); - dbbCreateFailed = true; - dbbAddressFailed = true; - return 0L; - } - dbbClass = c; - } - try { - m = c.getDeclaredMethod("address"); - } catch (NoSuchMethodException | SecurityException e) { - // throw new IllegalStateException("Could not get method DirectByteBuffer.address()", e); - dbbClass = null; - dbbAddressFailed = true; - return 0L; - } - m.setAccessible(true); - dbbAddressMethod = m; - } try { - return (Long) m.invoke(bb); - } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException e) { - // throw new IllegalStateException("Could not create an invoke DirectByteBuffer.address()", - // e); - dbbClass = null; - dbbAddressMethod = null; - dbbAddressFailed = true; + return unsafe.getBufferAddress(bb); + } catch (Exception e) { + // If Unsafe access fails, return 0 to indicate failure return 0L; } } /** - * Create a direct byte buffer given its address and size. The returned ByteBuffer will be direct - * and use the memory at the given address. + * Create a direct byte buffer given its address and size. + * + * This implementation creates a standard DirectByteBuffer and then modifies its internal + * 'address' field to point to the given memory address using Unsafe. This approach uses + * field-level access via Unsafe which does not require --add-opens flags. * - * @return the created direct byte buffer or null if it could not be created. + * The resulting ByteBuffer directly wraps the memory at the given address without copying, + * making it a zero-copy operation suitable for off-heap memory management. + * + * @param address the native memory address to wrap + * @param size the size of the buffer + * @return the created direct byte buffer wrapping the address, or null if creation failed */ - @SuppressWarnings({"rawtypes", "unchecked"}) static ByteBuffer createDirectByteBuffer(long address, int size) { - if (dbbCreateFailed) { + if (unsafe == null) { return null; } - Constructor ctor = dbbCtor; - if (ctor == null) { - Class c = dbbClass; - if (c == null) { - try { - c = Class.forName("java.nio.DirectByteBuffer"); - } catch (ClassNotFoundException e) { - // throw new IllegalStateException("Could not find java.nio.DirectByteBuffer", e); - dbbCreateFailed = true; - dbbAddressFailed = true; - return null; - } - dbbClass = c; - } - try { - ctor = c.getDeclaredConstructor(long.class, int.class); - } catch (NoSuchMethodException | SecurityException e) { - // throw new IllegalStateException("Could not get constructor DirectByteBuffer(long, int)", - // e); - dbbClass = null; - dbbCreateFailed = true; - return null; - } - ctor.setAccessible(true); - dbbCtor = ctor; - } try { - return (ByteBuffer) ctor.newInstance(address, size); - } catch (InstantiationException | IllegalAccessException | IllegalArgumentException - | InvocationTargetException e) { - // throw new IllegalStateException("Could not create an instance using DirectByteBuffer(long, - // int)", e); - dbbClass = null; - dbbCtor = null; - dbbCreateFailed = true; + // Allocate a small DirectByteBuffer using standard public API + // We'll reuse this buffer's structure but change its address and capacity + ByteBuffer buffer = ByteBuffer.allocateDirect(1); + + // Use Unsafe to modify the buffer's internal fields to point to our address + // This is similar to calling the private DirectByteBuffer(long, int) constructor + // but using field access instead of constructor reflection + unsafe.setBufferAddress(buffer, address); + unsafe.setBufferCapacity(buffer, size); + + // Reset position and limit to match the new capacity + buffer.clear(); + buffer.limit(size); + + return buffer; + } catch (Exception e) { return null; } } diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java index dbfc1ee400..2672c36b5d 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java @@ -27,7 +27,6 @@ import java.util.Collections; import java.util.List; import org.apache.geode.distributed.internal.deadlock.UnsafeThreadLocal; -import org.apache.geode.internal.offheap.AddressableMemoryManager; import org.apache.geode.internal.stats50.VMStats50; import org.apache.geode.unsafe.internal.com.sun.jmx.remote.security.MBeanServerAccessController; import org.apache.geode.unsafe.internal.sun.nio.ch.DirectBuffer; @@ -48,10 +47,6 @@ public class MemberJvmOptions { * open needed by {@link UnsafeThreadLocal} */ private static final String JAVA_LANG_OPEN = "--add-opens=java.base/java.lang=ALL-UNNAMED"; - /** - * open needed by {@link AddressableMemoryManager} - */ - private static final String JAVA_NIO_OPEN = "--add-opens=java.base/java.nio=ALL-UNNAMED"; /** * open needed by {@link VMStats50} */ @@ -62,8 +57,7 @@ public class MemberJvmOptions { COM_SUN_JMX_REMOTE_SECURITY_EXPORT, SUN_NIO_CH_EXPORT, COM_SUN_MANAGEMENT_INTERNAL_OPEN, - JAVA_LANG_OPEN, - JAVA_NIO_OPEN); + JAVA_LANG_OPEN); public static List<String> getMemberJvmOptions() { if (isJavaVersionAtLeast(JAVA_11)) { diff --git a/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/misc/Unsafe.java b/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/misc/Unsafe.java index 3c5db9e96d..96e0282055 100644 --- a/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/misc/Unsafe.java +++ b/geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/misc/Unsafe.java @@ -26,6 +26,33 @@ import java.lang.reflect.Field; public class Unsafe { private final sun.misc.Unsafe unsafe; + + // Cached field offsets for ByteBuffer access + // These are computed once and reused to avoid repeated reflection + private static final long BUFFER_ADDRESS_FIELD_OFFSET; + private static final long BUFFER_CAPACITY_FIELD_OFFSET; + + static { + long addressOffset = -1; + long capacityOffset = -1; + try { + Field unsafeField = sun.misc.Unsafe.class.getDeclaredField("theUnsafe"); + unsafeField.setAccessible(true); + sun.misc.Unsafe unsafeInstance = (sun.misc.Unsafe) unsafeField.get(null); + + // Get field offsets for Buffer fields + Field addressField = java.nio.Buffer.class.getDeclaredField("address"); + addressOffset = unsafeInstance.objectFieldOffset(addressField); + + Field capacityField = java.nio.Buffer.class.getDeclaredField("capacity"); + capacityOffset = unsafeInstance.objectFieldOffset(capacityField); + } catch (Exception e) { + // If initialization fails, offsets remain -1 + } + BUFFER_ADDRESS_FIELD_OFFSET = addressOffset; + BUFFER_CAPACITY_FIELD_OFFSET = capacityOffset; + } + { sun.misc.Unsafe tmp; try { @@ -210,4 +237,59 @@ public class Unsafe { public void putOrderedObject(Object o, long offset, Object x) { unsafe.putOrderedObject(o, offset, x); } + + /** + * Gets the native memory address from a DirectByteBuffer using field offset. + * This method accesses the 'address' field of java.nio.Buffer directly via Unsafe, + * which does not require --add-opens flags (unlike method reflection with setAccessible()). + * + * @param buffer the DirectByteBuffer to get the address from + * @return the native memory address + */ + public long getBufferAddress(Object buffer) { + if (BUFFER_ADDRESS_FIELD_OFFSET == -1) { + throw new RuntimeException("Buffer address field offset not initialized"); + } + return unsafe.getLong(buffer, BUFFER_ADDRESS_FIELD_OFFSET); + } + + /** + * Sets the native memory address for a ByteBuffer using field offset. + * This allows wrapping an arbitrary memory address as a ByteBuffer. + * + * @param buffer the ByteBuffer to set the address for + * @param address the native memory address + */ + public void setBufferAddress(Object buffer, long address) { + if (BUFFER_ADDRESS_FIELD_OFFSET == -1) { + throw new RuntimeException("Buffer address field offset not initialized"); + } + unsafe.putLong(buffer, BUFFER_ADDRESS_FIELD_OFFSET, address); + } + + /** + * Gets the capacity from a ByteBuffer using field offset. + * + * @param buffer the ByteBuffer to get the capacity from + * @return the buffer capacity + */ + public int getBufferCapacity(Object buffer) { + if (BUFFER_CAPACITY_FIELD_OFFSET == -1) { + throw new RuntimeException("Buffer capacity field offset not initialized"); + } + return unsafe.getInt(buffer, BUFFER_CAPACITY_FIELD_OFFSET); + } + + /** + * Sets the capacity for a ByteBuffer using field offset. + * + * @param buffer the ByteBuffer to set the capacity for + * @param capacity the capacity value + */ + public void setBufferCapacity(Object buffer, int capacity) { + if (BUFFER_CAPACITY_FIELD_OFFSET == -1) { + throw new RuntimeException("Buffer capacity field offset not initialized"); + } + unsafe.putInt(buffer, BUFFER_CAPACITY_FIELD_OFFSET, capacity); + } }
