Skip to content

Conversation

@JinwooHwang
Copy link
Contributor

For all changes, please confirm:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
  • Has your PR been rebased against the latest commit within the target branch (typically develop)?
  • Is your initial contribution a single, squashed commit?
  • Does gradlew build run cleanly?
  • Have you written or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

JinwooHwang and others added 9 commits November 30, 2025 18:42
Remove likely-to-become-outdated copies of release scripts to ensure
they are not run by accident from a branch (they should always be run
from develop).

Also remove CODEOWNERS to avoid the confusion of GitHub showing owner
names like on develop, but codeowner reviews not actually being
required (due to lack of branch protection or minimum review count on
support branches)
As part of the Geode Release Process, the build number must
be rolled forward so work can begin on the next release
…ernals (#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 Package (#7955)

* refactor: Replace internal JDK DirectBuffer with public API solution

Replace sun.nio.ch.DirectBuffer usage with BufferAttachmentTracker, using only
public Java APIs (WeakHashMap and ByteBuffer).

Changes:
- Created BufferAttachmentTracker: WeakHashMap-based tracker for slice-to-original
  buffer mappings, replacing internal DirectBuffer.attachment() access
- Updated BufferPool: Modified slice creation to record mappings and simplified
  getPoolableBuffer() to use the tracker
- Removed DirectBuffer wrapper: Deleted geode-unsafe DirectBuffer wrapper class
- Updated MemberJvmOptions: Removed SUN_NIO_CH_EXPORT from required JVM options
- Added comprehensive unit tests: BufferAttachmentTrackerTest validates all
  tracker functionality

Benefits:
- Eliminates one JVM module export requirement
- Uses only public Java APIs
- Maintains functionality with automatic memory cleanup via WeakHashMap
- Fully backward compatible

Testing:
- All BufferPool tests pass
- New BufferAttachmentTracker tests pass
- Compilation successful

* Add comprehensive documentation to BufferAttachmentTracker

- Add detailed PMD suppression justification explaining thread-safety
- Document why ConcurrentHashMap is safe for concurrent access
- Explain lock-free operations and atomic guarantees
- Add 7-line comment block explaining mutable static field design choice

* Apply spotless formatting to BufferAttachmentTrackerTest

* fix: Correct buffer pooling to prevent capacity issues in NioEngine

- Fixed acquirePredefinedFixedBuffer() to return full-capacity buffers
  instead of modifying buffer limits before return
- Added BufferAttachmentTracker.removeTracking() in releaseBuffer()
  to properly clean up slice-to-original mappings
- Created non-slicing buffer acquisition methods for NioPlainEngine
  and NioSslEngine which require reusable full-capacity buffers
- Separated buffer acquisition into two use cases:
  * Single-use sliced buffers (2-param acquireDirectBuffer)
  * Reusable full-capacity buffers (3-param acquireDirectBuffer)

This fixes IllegalArgumentException 'newLimit > capacity' errors in
distributed tests by ensuring pooled buffers maintain proper capacity.

* Fix IndexOutOfBoundsException in BufferAttachmentTracker

Replace ConcurrentHashMap with synchronized IdentityHashMap to avoid
ByteBuffer.equals() issues. ByteBuffer uses content-based equality which
can throw IndexOutOfBoundsException when buffer state (position/limit)
changes after being used as a map key. IdentityHashMap uses object
identity (==) which is safe and appropriate for tracking buffer relationships.
…mar (#7942)

* GEODE-10508: Fix ANTLR nondeterminism warnings in OQL grammar

This commit resolves four nondeterminism warnings generated by ANTLR during
the OQL grammar compilation process. These warnings indicated parser ambiguity
that could lead to unpredictable parsing behavior.

Problem Analysis:
-----------------
1. Lines 574 & 578 (projection rule):
   The parser could not distinguish between aggregateExpr and expr alternatives
   when encountering aggregate function keywords (sum, avg, min, max, count).
   These keywords are valid both as:
   - Aggregate function identifiers: sum(field)
   - Regular identifiers in expressions: sum as a field name

   Without lookahead, ANTLR could not deterministically choose which production
   rule to apply, resulting in nondeterminism warnings.

2. Lines 961 & 979 (aggregateExpr rule):
   Optional 'distinct' keyword created ambiguity in aggregate function parsing.
   The parser could not decide whether to:
   - Match the optional 'distinct' keyword, or
   - Skip it and proceed directly to the expression

   Both paths were valid, but ANTLR's default behavior doesn't specify
   preference, causing nondeterminism.

Solution Implemented:
--------------------
1. Added syntactic predicates to projection rule (lines 574, 578):
   Predicate: (('sum'|'avg'|'min'|'max'|'count') TOK_LPAREN)=>

   This instructs the parser to look ahead and check if an aggregate keyword
   is followed by a left parenthesis. If true, it chooses aggregateExpr;
   otherwise, it chooses expr. This resolves the ambiguity by providing
   explicit lookahead logic.

2. Added greedy option to aggregateExpr rule (lines 961, 979):
   Option: options {greedy=true;}

   This tells the parser to greedily match the 'distinct' keyword whenever
   it appears, rather than being ambiguous about whether to match or skip.
   The greedy option eliminates the nondeterminism by establishing clear
   matching priority.

3. Updated test to use token constants (AbstractCompiledValueTestJUnitTest):
   Changed: hardcoded value 89 -> OQLLexerTokenTypes.LITERAL_or

   Rationale: Adding syntactic predicates changes ANTLR's token numbering
   in the generated lexer (LITERAL_or shifted from 89 to 94). Using the
   constant ensures test correctness regardless of future grammar changes.
   This is a best practice for maintaining test stability.

Impact:
-------
- Zero nondeterminism warnings from ANTLR grammar generation
- No changes to OQL syntax or semantics (fully backward compatible)
- No runtime behavior changes (modifications only affect parser generation)
- All existing tests pass with updated token reference
- Improved parser determinism and maintainability

Technical Details:
-----------------
- Syntactic predicates (=>) are standard ANTLR 2 feature for lookahead
- Greedy option is standard ANTLR feature for optional subrule disambiguation
- Token constant usage follows best practices for generated code references
- Changes are compile-time only with no runtime performance impact

Files Modified:
--------------
- geode-core/src/main/antlr/org/apache/geode/cache/query/internal/parse/oql.g
- geode-core/src/test/java/org/apache/geode/cache/query/internal/AbstractCompiledValueTestJUnitTest.java

* GEODE-10508: Apply code formatting to test file

Fix line length formatting for improved readability.
…e System Encapsulation (#7954)

* Replace reflection-based UnsafeThreadLocal with WeakHashMap implementation

- Removed reflection access to ThreadLocal/ThreadLocalMap internals
- Implemented cross-thread value lookup using synchronized WeakHashMap
- Removed requirement for --add-opens=java.base/java.lang=ALL-UNNAMED
- WeakHashMap ensures terminated threads can be garbage collected
- Maintains same API and functionality for deadlock detection
- All existing tests pass without JVM flag changes

This eliminates the fragile reflection-based approach that required
special JVM flags and was vulnerable to Java module system changes.
The new implementation is safer, more maintainable, and works across
all Java versions without requiring internal access.

* Remove --add-opens=java.base/java.lang from test configuration

- Removed unnecessary JVM flag from geode-test.gradle line 185
- Flag no longer needed after UnsafeThreadLocal refactoring
- Tests now run with same security constraints as production
- All UnsafeThreadLocal and deadlock tests pass without the flag
- Validates that refactoring truly eliminated reflection dependency
…ncy Information (#7961)

* Correct license classification for Jakarta EE dependencies

- Moved jakarta.servlet v6.0.0 and jakarta.transaction v2.0.1 from CDDL to EPL 2.0 section
- These components use EPL 2.0 with GPL-2.0 + Classpath Exception, not CDDL 1.1

* GEODE-10511: Update istack-commons-runtime version from 4.0.1 to 4.1.1

- Aligns declared version with actual resolved version
- Eliminates version conflict resolution between 4.0.1 and 4.1.1
- Makes DependencyConstraints.groovy consistent with LICENSE file
- jaxb-core/jaxb-runtime 4.0.2 transitively requires 4.1.1

* GEODE-10511: Update test expectations for istack-commons-runtime 4.1.1

- Update geode-server-all dependency_classpath.txt
- Update geode-assembly assembly_content.txt to remove 4.0.1 reference
- Fixes integration test failures in both modules
…--add-opens Requirement (#7957)

* GEODE-10522: Eliminate reflection in VMStats50 to remove --add-opens requirement

Replace reflection-based access to platform MXBean methods with direct
interface casting, eliminating the need for
--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED JVM flag.

Key Changes:
- Replaced Method.invoke() with direct calls to com.sun.management interfaces
- Removed setAccessible(true) calls that required module opening
- Updated to use OperatingSystemMXBean and UnixOperatingSystemMXBean directly
- Removed COM_SUN_MANAGEMENT_INTERNAL_OPEN flag from MemberJvmOptions
- Removed unused ClassPathLoader import
- Improved code clarity and type safety

Benefits:
- Completes Java Platform Module System (JPMS) compliance initiative
- Eliminates last remaining --add-opens flag requirement
- Improves security posture (no module violations)
- Better performance (no reflection overhead)
- Simpler, more maintainable code

Testing:
- All VMStats tests pass
- Tested without module flags
- Uses public, documented APIs from exported com.sun.management package

This completes the module compliance initiative:
- GEODE-10519: Eliminated java.base/java.lang opening
- GEODE-10520: Eliminated sun.nio.ch export
- GEODE-10521: Eliminated java.base/java.nio opening
- GEODE-10522: Eliminated jdk.management/com.sun.management.internal opening (this commit)

Apache Geode now requires ZERO module flags to run on Java 17+.

* Apply code formatting to VMStats50

- Fix import ordering (move com.sun.management imports after java.util imports)
- Remove trailing whitespace
- Apply consistent formatting throughout

* Address reviewer feedback: Add null check and improve error message

- Add null check for platformOsBean before calling getAvailableProcessors()
- Enhance error message to clarify impact on statistics vs core functionality
- Both changes suggested by @sboorlagadda in PR review

* Remove SUN_NIO_CH_EXPORT reference from JAVA_11_OPTIONS

- Fix compilation error after merging GEODE-10520 changes
- SUN_NIO_CH_EXPORT constant was removed but still referenced in list

* Fix duplicate JAVA_NIO_OPEN and missing JAVA_LANG_OPEN

- Remove duplicate JAVA_NIO_OPEN definition
- Add missing JAVA_LANG_OPEN constant
- Fix comment to correctly reference UnsafeThreadLocal for JAVA_LANG_OPEN
…nd Java 17 (#7953)

* docs: Update documentation for Jakarta EE 10 and Java 17

- Update Java version format from 1.8.0_121 to 17.0.16
- Update all Geode module versions from 1.0.0 to 2.0.0
- Replace javax.transaction-api with jakarta.transaction-api 2.0.1
- Update dependency versions (slf4j 2.0.17, log4j 2.17.2, jgroups 3.6.20, fastutil 8.5.8)
- Update config.yml: min_java_version='17', min_java_update='16'
- Fix Java version template expressions across 20+ documentation files
- Update WebLogic HTTP session management guide for Jakarta EE 10
- Update installation guides with Java 17 requirements

Breaking Changes:
- Minimum Java version now Java 17.0.16 (was Java 8u121)
- Jakarta EE 10 required (was Java EE 8)
- All javax.* packages replaced with jakarta.*

Testing:
- Verified peer-to-peer and client-server configurations
- Documentation builds successfully
- All quality checks passed (spotlessCheck, rat, checkPom, pmdMain)

* docs: Address review feedback - fix version consistency and consolidate tc Server deprecation notes

- Fix Tomcat version inconsistency: Changed CATALINA_HOME path from 10.1.49 to 10.1.30 to match example text
- Consolidate duplicate tc Server removal messages into single Note for clarity
- Improve documentation consistency and readability

* Fix log file path to be consistent with server path
@JinwooHwang JinwooHwang closed this Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant