Conversation
Applications running the client on JDK 26 crash with IllegalAccessError the first time they send a double that needs the slow formatting path, such as 1e300, 4.9e-324 or Double.MAX_VALUE. Common values like 0.1 or 123456.789 are unaffected, so the failure shows up late and only for some rows. Building the client from source on JDK 26 fails outright. Cause: JDK 26 (JDK-8366017) made jdk.internal.math.FDBigInteger, which the client's double formatter borrows for its bignum arithmetic, package-private. No module export can make a non-public class reachable. Fix: the Java 9+ bridge now binds the eight FDBigInteger methods it needs through method handles resolved once at class-init, instead of naming the class in source. Existing --add-exports plumbing and the reflective module export become unnecessary and are removed. There is no performance cost: JMH on JDK 25 shows identical ns/op and B/op to the previous direct calls. Supported runtimes are unchanged: Java 8 through 26. The shipping JDK 8 artifact was verified on Java 8, 25 and 26; builds and tests were also run on JDK 11, 17, 25 and 26. A JMH benchmark for the double formatter is added under core/src/test. Fixes #96
Issue #96 passed every existing check because the JDK 8-built jar was only ever executed on JDK 8, 11 and 25, and source was only compiled on 8 and 25. The MRJAR smoke job is now a matrix over the JDKs the jar must run on (25, 26, plus a non-blocking 27-ea early warning) and the compile/javadoc smoke covers 25 and 26. The check names for JDK 25 are unchanged. JarPackagingIT also accepts QUESTDB_SMOKE_JDKS (path-separated JDK homes) so a developer with several JDKs installed gets the same cross-runtime check locally from `mvn install`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
Reproduced on Temurin 27+35: the Unsafe write to AccessibleObject.override no longer takes effect, so isAccessible() stays false and Lookup.unreflect cannot reach the package-private FDBigInteger. This defeats both this bridge and the old --add-exports export hack (same primitive). Only a launch-time --add-opens works there, which a library cannot impose on consumers; the durable JDK 27+ fix is a self-contained bignum. The non-blocking mrjar-smoke 27-ea CI job tracks this. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
…headers work JEP 450 compact object headers are default-on in JDK 27, shrinking the object header from 12 to 8 bytes and moving AccessibleObject.override from offset 12 to 8. Unsafe hard-coded 12/16, so the override write landed inside the header and setAccessible() silently no-opped -- the FdBig double-formatting bridge then threw IllegalAccessError on JDK 27 (proven: -XX:-UseCompactObjectHeaders makes the unchanged jar pass). AccessibleObject_override_fieldOffset() now measures the first-field boundary via a one-field probe instead of hard-coding a value; override sits at that boundary in every layout, so this tracks compact (8), compressed (12), uncompressed (16) and 32-bit (8) alike. Being in the shared source it also repairs every other Unsafe.makeAccessible call site under compact headers. Verified: the JDK 8 MRJAR formats all slow-path doubles on JDK 8/11/17/25/26 and 27-ea in both header modes. The mrjar-smoke 27-ea job is now a green forward canary. Updates the FdBig note accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
The compact-header layout that broke the FdBig bridge on JDK 27 was only exercised by the non-blocking 27-ea leg. A regression to a hard-coded override offset would pass every blocking job yet break users who enable -XX:+UseCompactObjectHeaders on GA JDK 25/26 (a product flag, no unlock) and all JDK 27 users. The mrjar-smoke step now runs DoubleFormatSmoke under three layouts on every leg: default, +UseCompactObjectHeaders, and -UseCompressedOops -UseCompressedClassPointers (offset 16). On the blocking 25/26 legs this turns the compact-header path into a blocking guard. +IgnoreUnrecognizedVMOptions keeps the flags harmless on any JDK. Verified locally: with the fix all three layouts pass on 25/26/27-ea; with the previous hard-coded offset the compact-headers leg fails on 25 and 26. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
The mrjar-smoke job now runs on JDK 25/26/27-ea under several object-header layouts, so two "on JDK 25" comments were false: merge the duplicated, contradictory comment block above the job into one, and update the DoubleFormatSmoke javadoc to describe the actual matrix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
Measured on JDK 25: unreflect returns a DirectMethodHandle, but asType(erased) wraps the six handles that narrow an Object parameter back to the bignum type in a cast-inserting BoundMethodHandle; only the two valueOf* handles (return-only widening) stay direct. No user consequence -- C2 inlines the chain and folds the casts, allocation is identical to a direct call -- but the sentence was wrong. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S8vZt12JbC8o8rGyHYTPrE
[PR Coverage check]😍 pass : 27 / 50 (54.00%) file detail
|
Review — level 3
Findings below were verified by execution, not by reading. CriticalNone. The mechanism is correct. The measured first-field offset tracks every layout (12 default / 8 compact / 16 uncompressed) on JDK 25 and 26; ModerateM1 — No CI job checks the rewritten java11 bridge against exact output
Reachability: Attribution, stated plainly: the blind spot itself is pre-existing. What this PR changes is the risk class it fails to cover — the old bridge was six lines of compiler-checked direct calls; the new one hand-writes eight Why Moderate, not Critical: the current code is proven correct (4 JDKs x 3 layouts), the loud mis-binding modes are caught by the smoke, and a real offset exists — Suggested fix (cheap, hardens everything at once): add an expected-string column to M2 — Shipped javadoc asserts a performance equality that measurement contradicts
The
Suggested fix: soften both claims to what is evidenced ("same allocation and, within measurement error, the same ns/op; ~1.4% more instructions per bridged call, plus a one-off handle-resolution cost at the first slow-path double"), and note that the A/B is only possible on JDK <= 25. Minor
CoverageTest gate passes. One admitted coverage gap (M1, Moderate); no Critical gap. The offset half of the fix is covered: mutation testing confirms no hard-coded constant (8/12/16) survives all three blocking smoke legs, and the matrix genuinely expands to three jobs with SummaryVerdict: approve with comments. Both gates pass. M1 is worth doing in this PR — a few lines, and it closes a blind spot this change makes newly load-bearing. M2 is a one-paragraph doc correction. Neither blocks. Severity distribution: 0 Critical, 2 Moderate, 4 Minor. All findings are in-diff; no out-of-diff breakage. The parent Tradeoff worth stating: the PR trades compile-time type checking for runtime handle resolution. That is unavoidable — no module export can reach a package-private class — and the implementation is correct. M1 is the compensating control that trade implies. Unrelated to this PR, but found while reviewing
|
Applications running the client on JDK 26 crash with IllegalAccessError the first time they send a double that needs the slow formatting path, such as 1e300, 4.9e-324 or Double.MAX_VALUE. Common values like 0.1 or 123456.789 are unaffected, so the failure shows up late and only for some rows. Building the client from source on JDK 26 fails outright.
Cause: JDK 26 (JDK-8366017) made jdk.internal.math.FDBigInteger, which the client's double formatter borrows for its bignum arithmetic, package-private. No module export can make a non-public class reachable.
Fix: the Java 9+ bridge now binds the eight FDBigInteger methods it needs through method handles resolved once at class-init, instead of naming the class in source. Existing --add-exports plumbing and the reflective module export become unnecessary and are removed. There is no performance cost: JMH on JDK 25 shows identical ns/op and B/op to the previous direct calls.
Supported runtimes are unchanged: Java 8 through 26. The shipping JDK 8 artifact was verified on Java 8, 25 and 26; builds and tests were also run on JDK 11, 17, 25 and 26. A JMH benchmark for the double formatter is added under core/src/test.
Fixes #96
Benchmark
DoubleFormatBenchmark(JMH 1.37, added in this PR) run against a client jar built frommain(directFDBigIntegercalls) and one from this branch (method handles), same JDK. Each shape is a fixed pool of 1024 doubles cycled per call; the slow-path share was measured by running the pool against the pre-fix jar on JDK 26, where the bignum path throws and the fast path does not.Machine: AMD Ryzen 9 9950X, Linux, Corretto 25.0.2. Timing: 2 forks x 5 warmup x 5 measurement iterations of 1 s.
mainns/op1.0,123456.789, ...)nextDouble())nextDouble() * 1e6)longBitsToDouble(random))1e300,4.9e-324,MAX_VALUE, ...)All deltas are within the run-to-run error. The slow-path rows (
wide,extreme) are the ones that exercise the new bridge on every call.Allocation per call (
-prof gc,gc.alloc.rate.norm, 1 fork):mainB/opIdentical to the byte. The handles are
static finaland invoked withinvokeExacton exact primitive signatures, so nothing boxes and the JIT inlines them to direct calls. The slow path's ~600 B/op is the JDK'sFDBigIntegerarithmetic plus the fourFdBigwrappers, unchanged from before.To reproduce, after
mvn -DskipTests package:JDK 27 (compact object headers)
The 27-ea smoke job initially failed at runtime even though the mechanism is sound. Root cause: JEP 450 compact object headers are enabled by default in JDK 27, shrinking the object header from 12 to 8 bytes and moving
AccessibleObject.overridefrom offset 12 to 8.Unsafehard-coded 12/16, so theoverridewrite landed inside the header andsetAccessible()silently did nothing. Disabling the feature (-XX:-UseCompactObjectHeaders) made the unchanged jar pass, confirming the cause.Fix:
Unsafe.AccessibleObject_override_fieldOffset()now measures the first-field boundary (via a one-field probe class) instead of hard-coding it.overridesits at that boundary in every layout, so this tracks compact (8), compressed (12), uncompressed (16) and 32-bit (8) automatically. Being in the shared source, it also repairs every otherUnsafe.makeAccessiblecall site under compact headers.Verified: the JDK 8-built jar formats all slow-path doubles on JDK 8, 11, 17, 25, 26 and 27-ea (both
+/-UseCompactObjectHeaders). Themrjar-smoke27-ea job is now a green forward canary (non-blocking, since EA is a moving target). The only remaining JDK-27+ risk is the eventual removal ofsun.misc.Unsafe; the durable answer then is a self-contained bignum with no JDK-internal access.