Skip to content

fix(build): restore compatibility with JDK11+ - #84

Merged
jerrinot merged 3 commits into
mainfrom
jh_jigsaw_compat
Aug 7, 2026
Merged

fix(build): restore compatibility with JDK11+#84
jerrinot merged 3 commits into
mainfrom
jh_jigsaw_compat

Conversation

@jerrinot

@jerrinot jerrinot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Fix crash on Java 11+ and module name regression introduced in 1.3.5

Who is affected: anyone using questdb-client 1.3.5 - 1.3.7 on Java 11 or newer, and anyone referencing the client from the module path.

Impact:

  • Ingesting certain double values (very large/small exponents) crashes with NoClassDefFoundError: sun/misc/FDBigInteger. Java 8 users are not affected.
  • The Java module name accidentally changed from io.questdb.client to questdb.client, breaking module-path consumers on upgrade.

This PR restores the module name and packages the jar so double formatting works on Java 8 and on Java 11 and newer. Java 9 and 10 (long-EOL, non-LTS) remain unsupported, as in every release since 1.3.4.

…oundError on Java 9+

The 1.3.5-1.3.7 releases moved the release build from JDK 11 to JDK 8,
which silently broke the published jar in two ways:

* module-info.class was no longer compiled and no Automatic-Module-Name
  was set, so the module name degraded from io.questdb.client to the
  filename-derived questdb.client.
* only the src/main/java8 FdBig bridge (sun.misc.FDBigInteger) was
  packaged; sun.misc.FDBigInteger does not exist on Java 9+, so
  slow-path double formatting (e.g. doubleColumn with extreme-exponent
  values) died with NoClassDefFoundError: sun/misc/FDBigInteger.

Fix, keeping JDK 8 as the release build JDK:

* pin Automatic-Module-Name: io.questdb.client in the jar manifest
* package the src/main/java11 bridge (jdk.internal.math.FDBigInteger)
  into META-INF/versions/11 and mark the jar Multi-Release: true; JDK 8
  builds compile it with a JDK 11+ from JAVA11_HOME (build fails fast
  if unset, instead of shipping a broken jar)
* JarPackagingIT (failsafe) asserts the packaged jar's manifest, MRJAR
  layout, and FdBig linkage, and executes slow-path double formatting
  against the jar in a child JVM - regressions that unit tests cannot
  see because they run against target/classes
* new CI job runs the JDK 8-built jar on JDK 25: checks the derived
  module name and the double-formatting smoke (DoubleFormatSmoke)
* enforcer refuses mvn deploy -P maven-central-publish on a non-JDK 8
  build, whose jar root would break Java 8 consumers

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jerrinot jerrinot added the bug Something isn't working label Aug 7, 2026
@jerrinot
jerrinot marked this pull request as ready for review August 7, 2026 14:55
Replace the hard-coded two-file javac invocation with a whole-directory
ant <javac>, so a file added to src/main/java11 cannot silently ship
without its versions/11 counterpart. JarPackagingIT now also cross-checks
every source in src/main/java11 against the packaged jar.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jerrinot jerrinot changed the title fix(build): restore compatibility with JDK9+ fix(build): restore compatibility with JDK11+ Aug 7, 2026
JarPackagingIT ran DoubleFormatSmoke only on the build JDK, so on JDK 8
builds -- including the release verify gate -- the META-INF/versions/11
classes were validated structurally but never executed; the only
executable proof lived in the CI smoke job, which never sees the
release-built jar. The IT now spawns a second child JVM from JAVA11_HOME
(always present during JDK 8 packaging) so the versioned bridge is run
against the exact jar being shipped.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jerrinot

jerrinot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I've verified the full chain. Here's my review.


PR #84 Review — fix(build): restore compatibility with JDK11+ (level 0)

Level 0 single-pass review. Level 3 exists for a full 13-agent mission-critical pass — reasonable to consider here since this touches the release/publish pipeline, but the diff is self-contained build config + tests and I was able to verify it end-to-end inline.

Committed-binary gate: PASS. All 6 changed files are text (git diff --numstat shows real line counts, no -/- binary markers). No build artifacts committed.

What this PR is: pure build/packaging + test infrastructure. No production client code changes. It (a) pins Automatic-Module-Name: io.questdb.client + Multi-Release: true in the JDK 8 jar manifest, (b) compiles src/main/java11 into META-INF/versions/11 via antrun on JDK 8 builds, (c) fails the build fast if JAVA11_HOME is absent, (d) refuses to publish from a non-JDK-8 JDK, and (e) adds DoubleFormatSmoke + JarPackagingIT to guard the packaged jar.

Verification performed

  • Test efficacy (the load-bearing check). Traced DoubleFormatSmoke's values through Numbers.appendDouble0: the FdBig slow path (Numbers.java:659) is taken when !(bBits < 64 && tenBits < 64). Subnormal/extreme-exponent values (Double.MIN_VALUE, -2.225073858507201E-308, Double.MAX_VALUE, 1.0E-300) push those bit counts ≥ 64 and genuinely reach FdBigjdk.internal.math.FDBigInteger. Without the versions/11 bridge these throw NoClassDefFoundError: sun/misc/FDBigInteger on Java 9+, so the smoke exits non-zero. The regression test is not vacuous — it reproduces the exact 1.3.5–1.3.7 bug.
  • Runtime access. Confirmed Compat.exportFdBigInteger() (pre-existing) reflectively calls Module.implAddExports so the smoke works under plain java -cp with no --add-exports — matching what real classpath consumers get.
  • No arch mismatch at release. All three JDK-8 packaging jobs that need it (ci.yml:build-jdk8, release verify, release publish) run on ubuntu-latest (x64) and set JAVA11_HOME: ${{ env.JAVA_HOME_11_X64 }}. The env var name matches setup-java's x64 export. No aarch64/mac packaging job is left without it.
  • mvn -pl core test still works without JAVA11_HOME — antrun is bound to prepare-package and failsafe to integration-test, both after test; confirmed matches the CLAUDE.md claim.
  • Surefire/failsafe split is correct: **/*IT.class excluded from surefire, failsafe's default **/*IT.java picks up JarPackagingIT, and it's the only *IT file (no pre-existing IT silently disabled).
  • Java 9/10 correctly stated as unsupportedversions/11 is not selected by a Java 9/10 runtime, which falls back to the root sun.misc class. Description is honest.

Critical

None.

Moderate

None.

Minor

  1. JarPackagingIT.collectJavaSources assumes filename == class name (JarPackagingIT.java:118). It maps each Foo.java → a single META-INF/versions/11/.../Foo.class and asserts that entry exists. It does not verify inner/nested classes (Foo$Inner.class) or a package-private top-level class whose name differs from the file. Harmless for the current FdBig.java/Compat.java (and antrun compiles the whole srcdir, so an outer class can't ship without its inner classes), but the guard is weaker than its comment implies ("every source in src/main/java11 must ship"). If someone later adds a java11 source with a differently-named top-level class the assertion would false-fail. Low priority.

  2. runSmokeAgainstJar never closes the child Process streams (JarPackagingIT.java:82). process.getInputStream() is read to EOF (correct order — avoids the pipe deadlock, and redirectErrorStream(true) means no second pipe), but the stream/process isn't closed on any path. Negligible for a short-lived test subprocess; noting for completeness.

  3. No Fixes #NNN in the PR body. If there's a tracking issue for the 1.3.5–1.3.7 regression, reference it at the top per CLAUDE.md conventions.

Informational (behavioral change worth surfacing)

  • Any JDK 8 mvn package/install/verify now hard-requires JAVA11_HOME, including the parent questdb repo's -P local-client build if it drives this module through packaging on JDK 8. This is intentional (fail-fast beats silently shipping the broken jar) and documented in the CLAUDE.md diff, but downstream/parent-repo builders should be aware.

Summary

Verdict: approve. Well-constructed, correctly-targeted fix with a genuinely efficacious regression test — I verified the test reaches the FdBig path that caused the original crash, and that all release packaging jobs have JAVA11_HOME wired with the correct arch suffix. 0 Critical, 0 Moderate, 3 Minor nits, 1 informational. Draft findings: I checked the two highest-risk hypotheses (vacuous smoke test; release-time arch mismatch leaving JAVA11_HOME empty) and both were disproven. All findings in-diff; cross-context surface is limited (the only production symbols touched by the tests — StringSink.put(double), Compat.exportFdBigInteger, FdBig — are unchanged and verified reachable).

@nwoolmer nwoolmer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L G T M

@jerrinot
jerrinot enabled auto-merge (squash) August 7, 2026 16:22
@jerrinot
jerrinot merged commit f0b7276 into main Aug 7, 2026
15 checks passed
@jerrinot
jerrinot deleted the jh_jigsaw_compat branch August 7, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants