Jinwoo Hwang created GEODE-10521:
------------------------------------
Summary: 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
Fix For: 2.1.0
h2. Summary
Apache Geode currently accesses the internal {{sun.nio.ch.DirectBuffer}}
interface to track buffer slice-to-original mappings in {{{}BufferPool{}}}.
This requires the JVM export flag
{{{}--add-exports=java.base/sun.nio.ch=ALL-UNNAMED{}}}, creating security
vulnerabilities, operational complexity, and maintenance risks. This access
violates Java Platform Module System (JPMS) encapsulation and depends on
non-public APIs that could change or be removed in future JDK releases.
h2. Problem Description
h3. Current Implementation Dependencies
The {{BufferPool}} class in
{{geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java}}
manages ByteBuffer pooling for network communication. When creating buffer
slices, it needs to track the relationship between slices and their original
buffers to support proper buffer pooling and reuse.
*Current approach:*
* Uses {{sun.nio.ch.DirectBuffer}} internal interface
* Calls {{DirectBuffer.attachment()}} method to retrieve the original buffer
from a slice
* Requires a wrapper class in {{geode-unsafe}} module to access this internal
API
* Necessitates JVM export flag:
{{--add-exports=java.base/sun.nio.ch=ALL-UNNAMED}}
*Code location:*
{{geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java}}
{code:java}
// Current implementation accessing internal API
import org.apache.geode.unsafe.internal.sun.nio.ch.DirectBuffer;
// In BufferPool class
DirectBuffer directBuffer = (DirectBuffer) buffer;
ByteBuffer attachment = (ByteBuffer) directBuffer.attachment();
{code}
h3. Required JVM Configuration
To enable this internal API access, Geode requires the following JVM export
flag (defined in {{{}MemberJvmOptions.java{}}}):
{code:java}
/**
* export needed by DirectBuffer wrapper
*/
private static final String SUN_NIO_CH_EXPORT =
"--add-exports=java.base/sun.nio.ch=ALL-UNNAMED";
{code}
This flag must be configured on all Geode server JVMs, adding to the
operational burden alongside other required flags for {{java.lang}} and
{{java.nio}} access.
h3. Wrapper Module Requirements
The {{geode-unsafe}} module contains a wrapper class to access the internal
DirectBuffer interface:
{{geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBuffer.java}}
This wrapper:
* Duplicates the internal interface definition
* Creates tight coupling to JDK internals
* Requires maintenance when JDK internal structures change
* Adds complexity to the build and dependency management
h2. Problems with Current Approach
h3. 1. Java Module System Violations
*Severity: High*
The current approach directly violates the Java Platform Module System (JPMS)
encapsulation:
* Accesses internal implementation details of {{sun.nio.ch}} package
* Bypasses the module system's access controls
* Requires explicit {{--add-exports}} directive to override module boundaries
* Opens entire {{sun.nio.ch}} package to all unnamed modules
{*}Risk{*}: Future Java versions may further restrict or remove the ability to
export internal packages, even with command-line flags. The {{sun.*}} packages
are explicitly documented as internal and subject to change without notice.
h3. 2. Maintenance Burden and Fragility
*Severity: High*
The implementation is brittle and creates ongoing maintenance costs:
* Internal interface definitions may change between JDK releases
* Method signatures in {{sun.nio.ch.DirectBuffer}} could be modified
* The {{attachment()}} mechanism could be removed or redesigned
* Wrapper class must be manually synchronized with JDK changes
* No compiler warnings when internal APIs change
{*}Historical precedent{*}: Oracle has removed or modified internal APIs in
past JDK releases, breaking code that depended on them. The {{sun.*}} namespace
carries no stability guarantees.
h3. 3. Security Concerns
*Severity: Medium-High*
Using {{{}--add-exports=java.base/sun.nio.ch=ALL-UNNAMED{}}}:
* Opens internal NIO channel implementation to all unnamed modules
* Grants broader access than necessary (only need buffer attachment tracking)
* Weakens Java's security model and module encapsulation
* May trigger security audit flags in enterprise environments
* Creates potential attack vectors by exposing internal implementation
{*}Security implications{*}:
* Internal {{sun.nio.ch}} classes handle low-level I/O operations
* Exposing these internals could enable manipulation of channel state
* Buffer management internals could be exploited if exposed
* Violates principle of least privilege
h3. 4. Operational Complexity
*Severity: Medium*
Current deployment requirements:
* JVM export flags must be correctly configured on all Geode servers
* Different flag configurations needed for different Java versions
* Documentation and deployment scripts must track required flags
* Missing flags cause runtime failures with cryptic error messages
* Troubleshooting is complicated by module system violations
>From {{{}MemberJvmOptions.java{}}}, the growing list of required flags:
{code:java}
static final List<String> JAVA_11_OPTIONS = Arrays.asList(
COM_SUN_JMX_REMOTE_SECURITY_EXPORT,
SUN_NIO_CH_EXPORT, // ← Required for DirectBuffer access
COM_SUN_MANAGEMENT_INTERNAL_OPEN,
JAVA_LANG_OPEN,
JAVA_NIO_OPEN);
{code}
Each additional flag increases:
* Configuration complexity
* Risk of misconfiguration
* Deployment documentation burden
* Operational support costs
h3. 5. Build and Dependency Complexity
*Severity: Low-Medium*
The {{geode-unsafe}} module approach:
* Adds extra module to build and maintain
* Creates circular dependency concerns
* Complicates IDE setup and development workflow
* Increases build time and complexity
* Requires special handling in build tools
h3. 6. Compliance and Audit Issues
*Severity: Medium*
Enterprise environments face challenges:
* Security audits flag usage of internal APIs
* Compliance reviews require justification for module violations
* Risk assessment must account for non-public API dependencies
* Change management processes are triggered by JDK upgrades
* Documentation burden for security exception approvals
h2. Business Impact
h3. Reduced Security Risk
Eliminating {{sun.nio.ch}} access:
* Removes need to export internal NIO channel implementation
* Reduces attack surface by not exposing low-level I/O internals
* Simplifies security audits and compliance reviews
* Follows principle of least privilege
* Eliminates security exception requirements
h3. Improved Operational Simplicity
Removing the JVM export flag requirement:
* Simplifies deployment procedures
* Reduces configuration errors in production
* Removes a potential point of failure during Java upgrades
* Improves compatibility with container orchestration platforms
* Reduces operational support burden
h3. Better Java Version Compatibility
A solution using only public APIs:
* Works seamlessly across all Java versions (17, 21+)
* Doesn't depend on JDK internal implementation details
* Reduces risk during major Java version upgrades
* Aligns with Java's "code to interfaces, not implementations" principle
* Protects against breaking changes in future JDKs
h3. Enhanced Long-Term Maintainability
Moving away from internal APIs:
* Eliminates the {{geode-unsafe}} wrapper module
* Reduces technical debt significantly
* Makes the codebase more sustainable
* Demonstrates modern Java best practices
* Simplifies onboarding for new contributors
h3. Reduced Build Complexity
Removing {{geode-unsafe}} module:
* Simplifies build configuration
* Reduces module interdependencies
* Improves IDE development experience
* Faster build times
* Cleaner dependency graph
h2. Scope
h3. Affected Components
*Source Files*
* {{geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java}} -
BufferPool class (primary)
*
{{geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/MemberJvmOptions.java}}
- JVM flags
*
{{geode-unsafe/src/main/java/org/apache/geode/unsafe/internal/sun/nio/ch/DirectBuffer.java}}
- Wrapper (to be removed)
*Test Files*
* {{geode-core/src/test/java/.../BufferPoolTest.java}} - Buffer pooling tests
* {{geode-unsafe/src/test/java/.../DirectBufferTest.java}} - Wrapper tests (to
be removed)
h3. Scope
* Analysis of buffer slice-to-original mapping requirements
* Evaluation of public API alternatives (WeakHashMap, custom tracking)
* Assessment of memory management implications
* Performance impact analysis
* Compatibility evaluation with existing BufferPool users
h2. Technical Constraints and Requirements
h3. Functional Requirements
* {*}Buffer tracking{*}: Must maintain ability to map buffer slices back to
their original buffers
* {*}Memory management{*}: Must support proper cleanup when buffers are no
longer needed
* {*}Thread safety{*}: Must work correctly in multi-threaded network I/O
scenarios
* {*}Performance{*}: Must not significantly impact network I/O performance
* {*}Backward compatibility{*}: Must maintain existing BufferPool API
h3. Non-Functional Requirements
* {*}Security{*}: Use only public Java APIs
* {*}Maintainability{*}: Solution should be simple and well-documented
* {*}Portability{*}: Work across all supported Java versions (11, 17, 21)
* {*}Memory efficiency{*}: No memory leaks from buffer tracking
* {*}Operational simplicity{*}: No special JVM flags required
h2. Related Work
h3. Similar Issues in Geode
This is part of a broader initiative to eliminate internal API usage:
* GEODE-10519: UnsafeThreadLocal accesses {{java.lang}} internals (requires
{{{}--add-opens{}}})
* AddressableMemoryManager: Accesses {{java.nio}} internals (requires
{{{}--add-opens{}}})
* Other {{sun.{*}{*}}} *and {{com.sun.}}* dependencies requiring module exports
h3. Module System Compliance Initiative
Goal: Make Apache Geode fully compliant with Java Platform Module System:
* Eliminate all {{-{-}add-opens{-}}} and {{-add-exports}} requirements
* Use only public, stable Java APIs
* Improve security posture
* Simplify deployment and operations
* Future-proof against JDK changes
h2. Priority Assessment
*Recommended Priority: Medium-High*
h3. Factors Supporting High Priority
* Security vulnerability from exposing internal NIO implementation
* Maintenance risk from dependency on unstable internal APIs
* Operational complexity from JVM flag requirements
* Compliance issues in enterprise environments
* Technical debt that grows with each JDK release
h3. Factors Supporting Medium Priority
* Current implementation works (with export flags)
* Not causing immediate production failures
* Workaround available (configure JVM flags)
* Other similar issues also need addressing
* Requires careful testing of buffer management
h3. Risk Assessment
{*}If not addressed{*}:
* Risk of breakage in future JDK releases (High probability)
* Continued security audit concerns (Medium impact)
* Ongoing operational complexity (Medium impact)
* Accumulating technical debt (High impact over time)
h2. Success Criteria
* No {{--add-exports=java.base/sun.nio.ch=ALL-UNNAMED}} flag required
* {{geode-unsafe}} module eliminated (or DirectBuffer wrapper removed)
* All buffer pooling functionality maintained
* No performance regression in network I/O
* Works on Java 17, and 21 without special configuration
* Code uses only public Java APIs
* Solution is well-documented and maintainable
h2. Additional Context
h3. Why This Matters
# {*}Security{*}: Exposing {{sun.nio.ch}} internals weakens module boundaries
for critical I/O code
# {*}Maintainability{*}: Internal NIO APIs have changed in past JDK releases,
breaking dependent code
# {*}Compliance{*}: Enterprise security reviews flag internal API usage as
high-risk
# {*}Simplicity{*}: Removing JVM export requirements reduces operational burden
h3. Historical Context
The original {{BufferPool}} implementation was created before the Java module
system existed. When JPMS was introduced in Java 9:
* Internal APIs became encapsulated in modules
* {{sun.*}} packages required explicit export flags
* The {{attachment()}} pattern became a module violation
* A wrapper class was created to isolate the internal access
With modern Java (11+), this approach is no longer sustainable:
* Security concerns have increased
* Module system enforcement is stricter
* Future JDK versions may block internal access entirely
* Better public API solutions are available
h3. Technical Background
*ByteBuffer slice tracking:*
When {{ByteBuffer.slice()}} creates a view of a buffer:
* The slice shares the same backing memory
* The slice doesn't inherently know its "parent" buffer
* The internal {{DirectBuffer.attachment()}} was used to track this
relationship
* This relationship is needed for proper buffer pooling and reuse
*Why tracking is needed:*
* BufferPool manages a pool of large buffers
* Small slices are created for individual messages
* When a slice is released, the original buffer must be returned to the pool
* Proper tracking prevents memory leaks and enables buffer reuse
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
