[ 
https://issues.apache.org/jira/browse/GEODE-10521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18043618#comment-18043618
 ] 

ASF subversion and git services commented on GEODE-10521:
---------------------------------------------------------

Commit 2819823b0782634cf8543a2f7f91328d00a9e442 in geode's branch 
refs/heads/support/2.0 from Jinwoo Hwang
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=2819823b07 ]

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.

> Eliminate Reflection-Based Access to java.nio.Buffer Internals in 
> AddressableMemoryManager
> ------------------------------------------------------------------------------------------
>
>                 Key: GEODE-10521
>                 URL: https://issues.apache.org/jira/browse/GEODE-10521
>             Project: Geode
>          Issue Type: Improvement
>            Reporter: Jinwoo Hwang
>            Assignee: Jinwoo Hwang
>            Priority: Major
>             Fix For: 2.1.0
>
>
> h2. Problem Description
> h3. Current State
> The {{AddressableMemoryManager}} class in {{geode-core}} currently uses 
> reflection to access private internal APIs of 
> {{{}java.nio.DirectByteBuffer{}}}:
>  # {{DirectByteBuffer.address()}} method - to retrieve the native memory 
> address from a DirectByteBuffer
>  # {{DirectByteBuffer(long, int)}} constructor - to create a DirectByteBuffer 
> wrapping a native memory address
> This reflection-based approach requires calling {{setAccessible(true)}} on 
> both the method and constructor, which violates Java's module encapsulation 
> rules introduced in Java 9 (JEP 260, 261).
> h3. Required JVM Flag
> To allow this reflection at runtime, Apache Geode currently requires the 
> following JVM flag:
> {code:java}
> --add-opens=java.base/java.nio=ALL-UNNAMED
> {code}
> This flag is hardcoded in {{MemberJvmOptions.JAVA_11_OPTIONS}} as 
> {{JAVA_NIO_OPEN}} and automatically added to all Geode member JVMs running on 
> Java 11 and later.
> h3. Code Evidence
> Current implementation in {{{}AddressableMemoryManager.java{}}}:
> {code:java}
> // Reflection-based access requiring --add-opens
> private static volatile Method dbbAddressMethod = null;
> private static volatile Constructor dbbCtor = null;
> public static long getDirectByteBufferAddress(ByteBuffer bb) {
>     Method m = dbbAddressMethod;
>     if (m == null) {
>         m = dbbClass.getDeclaredMethod("address");
>         m.setAccessible(true);  // REQUIRES --add-opens
>         dbbAddressMethod = m;
>     }
>     return (Long) m.invoke(bb);
> }
> static ByteBuffer createDirectByteBuffer(long address, int size) {
>     Constructor ctor = dbbCtor;
>     if (ctor == null) {
>         ctor = dbbClass.getDeclaredConstructor(long.class, int.class);
>         ctor.setAccessible(true);  // REQUIRES --add-opens
>         dbbCtor = ctor;
>     }
>     return (ByteBuffer) ctor.newInstance(address, size);
> }
> {code}
> h2. Business Impact
> h3. Security and Compliance Issues
> h4. Java Platform Security Model Violations
> The use of {{--add-opens}} flags represents a security concern and technical 
> debt:
>  * {*}Strong Encapsulation Bypass{*}: The flag explicitly breaks Java's 
> module system encapsulation, which was designed to protect internal 
> implementation details
>  * {*}Security Manager Conflicts{*}: In environments with security managers 
> or restricted permissions, these flags may be blocked or flagged as security 
> violations
>  * {*}Audit Trail Issues{*}: Security audits and compliance scans flag 
> {{--add-opens}} usage as potential vulnerabilities
>  * {*}Attack Surface{*}: Opening internal packages to ALL-UNNAMED increases 
> the attack surface by allowing any code to access these internals
> h4. Deployment and Operations Impact
> The required JVM flag creates operational friction:
>  * {*}Container Security Policies{*}: Many container platforms (Kubernetes, 
> Docker Enterprise) have security policies that restrict or audit JVM flags
>  * {*}Cloud Platform Restrictions{*}: Some cloud platforms (AWS Lambda, Azure 
> Functions, GCP Cloud Run) restrict or monitor JVM flags
>  * {*}Enterprise Security Policies{*}: Enterprise security teams often 
> require justification and approval for {{--add-opens}} flags
>  * {*}Deployment Complexity{*}: Administrators must remember to include the 
> flag in all deployment configurations
> h4. Forward Compatibility Risk
> Java's module system evolution poses risks:
>  * {*}JEP 403 (Java 17+){*}: Strongly encapsulates JDK internals by default, 
> with warnings issued for illegal reflective access
>  * {*}Future Java Versions{*}: The Java platform is moving toward stricter 
> enforcement of module boundaries
>  * {*}Deprecation Path{*}: {{--add-opens}} may become deprecated or 
> restricted in future Java releases
>  * {*}Migration Cost{*}: The longer this technical debt remains, the more 
> costly it becomes to remediate
> h3. Technical Impact
> h4. Off-Heap Memory Management
> {{AddressableMemoryManager}} is a critical component in Geode's off-heap 
> memory management:
>  * {*}Purpose{*}: Manages direct ByteBuffer access to off-heap memory regions
>  * {*}Performance Critical{*}: Used in hot paths for off-heap data access
>  * {*}Memory Safety{*}: Provides abstraction layer over native memory 
> addresses
>  * {*}Zero-Copy Operations{*}: Enables wrapping native memory addresses as 
> ByteBuffers without copying data
> Current usage patterns:
> {code:java}
> // Get address from a DirectByteBuffer
> long address = AddressableMemoryManager.getDirectByteBufferAddress(buffer);
> // Create a ByteBuffer wrapping a native memory address
> ByteBuffer wrapper = AddressableMemoryManager.createDirectByteBuffer(address, 
> size);
> {code}
> h4. Dependency Chain
> The {{java.nio}} module opening affects multiple components:
>  * {{{}geode-core{}}}: Direct usage in {{AddressableMemoryManager}}
>  * {{{}geode-gfsh{}}}: Automatic flag injection via {{MemberJvmOptions}}
>  * All Geode members: Every server, locator, and client requires the flag
>  * Test infrastructure: All integration and unit tests must include the flag
> h2. Benefits of Remediation
> h3. Security and Compliance
>  * {*}Eliminates Security Audit Findings{*}: Removes {{--add-opens}} flag 
> from security scan results
>  * {*}Improves Security Posture{*}: Operates within Java's intended security 
> model
>  * {*}Simplifies Compliance{*}: Reduces justification burden for security 
> teams
>  * {*}Future-Proof Security{*}: Aligns with Java platform security evolution
> h3. Operational Excellence
>  * {*}Simplified Deployment{*}: One less JVM flag to configure and document
>  * {*}Cloud-Native Friendly{*}: Compatible with restrictive cloud platform 
> policies
>  * {*}Container Security{*}: Passes stricter container security policies 
> without exceptions
>  * {*}Reduced Configuration Complexity{*}: Fewer deployment-specific 
> configurations to maintain
> h3. Java Platform Compatibility
>  * {*}Java 17+ Ready{*}: Fully compatible with JEP 403 strong encapsulation
>  * {*}Java 21 Forward Compatible{*}: No deprecation warnings or compatibility 
> issues
>  * {*}Virtual Threads Compatible{*}: Works correctly with Project Loom 
> virtual threads
>  * {*}GraalVM Native Image Ready{*}: Eliminates reflection that complicates 
> native image generation
> h3. Developer Experience
>  * {*}Cleaner Code{*}: Less reflection boilerplate and error handling
>  * {*}Better IDE Support{*}: IDEs can analyze code without reflection 
> indirection
>  * {*}Easier Testing{*}: Tests don't require special JVM flag configurations
>  * {*}Reduced Maintenance{*}: Eliminates reflection-related failure modes
> h3. Performance Considerations
>  * {*}Faster Reflection{*}: Field offset caching is more efficient than 
> method reflection
>  * {*}JIT Optimization{*}: Direct Unsafe field access optimizes better than 
> reflective method invocation
>  * {*}Reduced Overhead{*}: Eliminates {{invoke()}} overhead on every call
>  * {*}Better Inlining{*}: JIT compiler can inline field access more 
> aggressively than reflection
> h2. Module System Context
> This issue is part of a broader initiative to eliminate all JVM module system 
> violations in Apache Geode:
> h3. Related Work
>  * {*}GEODE-10519{*}: Eliminated 
> {{--add-opens=java.base/java.lang=ALL-UNNAMED}} ({{{}UnsafeThreadLocal{}}})
>  * {*}GEODE-10520{*}: Eliminated 
> {{--add-exports=java.base/sun.nio.ch=ALL-UNNAMED}} ({{{}DirectBuffer{}}})
>  * {*}GEODE-10521{*}: This issue - Eliminate 
> {{--add-opens=java.base/java.nio=ALL-UNNAMED}} 
> ({{{}AddressableMemoryManager{}}})
>  * {*}Future{*}: 
> {{--add-opens=java.base/com.sun.management.internal=ALL-UNNAMED}} 
> ({{{}VMStats50{}}})
> h3. Strategic Goal
> The ultimate goal is to run Apache Geode on Java 17, and 21 without requiring 
> any {{-{-}add-opens{-}}} or {{-add-exports}} flags, ensuring:
>  * Full compliance with Java module system
>  * Maximum security and encapsulation
>  * Forward compatibility with future Java releases
>  * Cloud-native and container-friendly deployment
> h2. Affected Versions
>  * All Geode versions running on Java 17 and later
>  * Particularly impacts Java 17+ deployments where strong encapsulation 
> warnings appear
> h2. References
>  * [JEP 260: Encapsulate Most Internal APIs|https://openjdk.org/jeps/260]
>  * [JEP 261: Module System|https://openjdk.org/jeps/261]
>  * [JEP 403: Strongly Encapsulate JDK Internals|https://openjdk.org/jeps/403]
>  * Java Platform Module System (JPMS) specification
> h2. Additional Context
> h3. Why Reflection Was Used Initially
> The original implementation used reflection because:
>  # {{DirectByteBuffer}} constructor is package-private (not accessible)
>  # {{DirectByteBuffer.address()}} method is package-private (not accessible)
>  # Public {{ByteBuffer}} API doesn't expose native memory address
>  # No public API exists to wrap an arbitrary memory address as a ByteBuffer
> This was a pragmatic solution when Java 9 was new and the module system 
> implications were less understood.
> h3. Off-Heap Memory Architecture
> Geode's off-heap memory management is critical for large heap scenarios:
>  * Reduces garbage collection pressure by keeping data off-heap
>  * Enables multi-gigabyte caches without GC pauses
>  * Provides direct memory access for serialization/deserialization
>  * Supports memory-mapped file operations
> {{AddressableMemoryManager}} is a cornerstone of this architecture, making 
> its security and compatibility critical.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to