Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate by andimarek · Pull Request #4296 · graphql-java/graphql-java · GitHub

Summary

Fix a race condition in CompletionStageSubscriber.onComplete() where the completion signal could be lost if an in-flight CompletionStage resolved concurrently, leaving downstream subscribers hanging indefinitely
Add jcstress stress tests to verify the fix under real concurrency pressure, integrated into both PR and master CI workflows
Fix flaky optional_spec111 TCK multicast test by using a shared single-thread executor per publisher
Add a post-run verification step that asserts exactly 49 TestNG TCK tests are skipped, catching any newly-silenced failure
Add per-class coverage gate to CI that fails on any class regressing any metric by >0.05%
Fix coverage gate false positives: skip classes with 0 total lines (interfaces, annotations, abstract classes) caused by JaCoCo non-determinism
Add tests for GraphQLList.equals()/hashCode() and CompletionStageOrderedSubscriber.cfExceptionUnwrap() error path

Details

Race condition fix

The race occurred in CompletionStageSubscriber when onComplete() checked inFlightDataQ.isEmpty() and decided to signal downstream, but between that check and setting onCompleteRun, another thread could drain the last CF and miss the runnable — leaving the subscriber hanging forever. Fix: extend the existing ReentrantLock scope to make the emptiness check and onCompleteRun set/claim a single atomic operation on both sides.

jcstress integration

jcstress doesn't produce JUnit XML, so console output is parsed to extract test counts (planned, passed, failed, soft errs, hard errs) for the unified test report.

Coverage gate

The per-class gate compares each class's line/branch/method coverage against the master baseline. Classes with 0 total lines (interfaces, annotations) are skipped to avoid false positives from JaCoCo non-deterministic attribution on non-concrete classes. The master workflow also excludes these from future baselines.

Missing test coverage

GraphQLList.equals()/hashCode(): explicit identity-semantics test (previously only covered non-deterministically via HashSet collisions)
CompletionStageOrderedSubscriber.cfExceptionUnwrap(): tests the catch block in emptyInFlightQueueIfWeCan() using a SilentlyFailedFuture that simulates the race where a CF has failed but its whenComplete callback hasn't fired yet

Test plan

Verify jcstress matrix job runs successfully in CI
Verify jcstress results appear in the PR test report comment
Verify existing Java 11/17/21/25 test jobs are unaffected
Verify per-class coverage gate passes (no false positives from empty-counter classes)
Verify new GraphQLList and CompletionStageOrderedSubscriber tests pass
Verify TCK skipped-test count verification passes

🤖 Generated with Claude Code

The spec317_mustSupportACumulativePendingElementCountUpToLongMaxValue TCK test intermittently hangs on CI (seen on Java 21). Root cause: a race between onComplete() and finallyAfterEachPromiseFinishes() where the queue-emptiness check and the onCompleteRun read/write are not atomic — the last CF can complete between the check and the set, leaving the runnable stranded forever. Fix by extending the existing ReentrantLock scope to also cover onCompleteRun, making the emptiness check and runnable set/claim a single atomic operation on both sides. Runnables are executed outside the lock to avoid deadlocks with user code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Test Report

Test Results

Java Version	Total	Passed	Failed	Errors	Skipped
Java 11	5671 (+2 🟢)	5614 (+5 🟢)	0 (±0)	0 (±0)	57 (-3)
Java 17	5671 (+2 🟢)	5613 (+5 🟢)	0 (±0)	0 (±0)	58 (-3)
Java 21	5671 (+2 🟢)	5613 (+5 🟢)	0 (±0)	0 (±0)	58 (-3)
Java 25	5671 (+2 🟢)	5613 (+6 🟢)	0 (±0)	0 (±0)	58 (-4)
jcstress	32 (+32 🟢)	32 (+32 🟢)	0 (±0)	0 (±0)	0 (±0)
Total	22716 (+40 🟢)	22485 (+53 🟢)	0 (±0)	0 (±0)	231 (-13)

Code Coverage (Java 25)

Metric	Covered	Missed	Coverage	vs Master
Lines	28699	3125	90.2%	+0.3% 🟢
Branches	8333	1509	84.7%	+1.3% 🟢
Methods	7681	1224	86.3%	+0.2% 🟢

Changed Class Coverage (59 classes)

Class	Line	Branch	Method
g.a.FieldComplexityEnvironment	+33.0% 🟢	±0.0%	+6.3% 🟢
g.a.QueryVisitorFieldEnvironmentImpl	-3.7% 🔴	+45.8% 🟢	-2.4% 🔴
g.a.QueryVisitorFragmentDefinitionEnvironmentImpl	+34.0% 🟢	±0.0%	+17.1% 🟢
g.a.QueryVisitorFragmentSpreadEnvironment	+30.0% 🟢	±0.0%	+22.9% 🟢
g.a.QueryVisitorInlineFragmentEnvironmentImpl	+27.8% 🟢	±0.0%	+17.1% 🟢
g.c.ImmutableMapWithNullValues	+2.8% 🟢	±0.0%	+2.9% 🟢
g.e.DataFetcherResult	+2.1% 🟢	+14.3% 🟢	-1.4% 🔴
g.e.ExecutionId	+8.3% 🟢	-50.0% 🔴	+16.7% 🟢
g.e.i.d.ExhaustedDataLoaderDispatchStrategy	+1.2% 🟢	+7.7% 🟢	±0.0%
g.e.MergedField	-0.3% 🔴	+3.6% 🟢	-1.4% 🔴
g.e.MergedField $MultiMergedField	+13.2% 🟢	+25.0% 🟢	-2.5% 🔴
g.e.NonNullableFieldWasNullError	+18.2% 🟢	±0.0%	+25.0% 🟢
g.e.r.CompletionStageOrderedSubscriber	+22.2% 🟢	+33.3% 🟢	+20.0% 🟢
g.e.r.CompletionStageSubscriber	+0.2% 🟢	+1.9% 🟢	+0.2% 🟢
g.e.ResultPath	-0.8% 🔴	+0.8% 🟢	-1.7% 🔴
g.GraphQLContext	+14.3% 🟢	+75.0% 🟢	+7.7% 🟢
g.GraphQLError $Builder	+11.3% 🟢	±0.0%	+14.1% 🟢
g.i.DeferPayload	±0.0%	-62.5% 🔴	±0.0%
g.i.IncrementalExecutionResult	+1.0% 🟢	+15.0% 🟢	-12.7% 🔴
g.i.StreamPayload	-5.4% 🔴	-62.5% 🔴	-8.3% 🔴
g.InvalidSyntaxError	-1.1% 🔴	±0.0%	-4.0% 🔴
g.l.IgnoredChar	+7.9% 🟢	-50.0% 🔴	-8.6% 🔴
g.l.SourceLocation	+6.5% 🟢	+1.4% 🟢	±0.0%
g.n.ExecutableNormalizedOperationToAstCompiler $ExecutionFragmentDetails	+18.2% 🟢	-50.0% 🔴	±0.0%
g.n.NormalizedInputValue	+6.9% 🟢	+18.7% 🟢	+6.7% 🟢
g.r.Connection	+7.1% 🟢	-70.0% 🔴	-8.3% 🔴
g.r.DefaultConnection	±0.0%	-70.0% 🔴	-16.7% 🔴
g.r.DefaultConnectionCursor	-1.3% 🔴	-12.5% 🔴	-13.3% 🔴
g.r.DefaultPageInfo	-12.1% 🔴	-78.6% 🔴	-20.8% 🔴
g.r.InvalidCursorException	+16.7% 🟢	±0.0%	+16.7% 🟢
g.s.d.EditOperation	+25.9% 🟢	±0.0%	+13.3% 🟢
g.s.FieldCoordinates	+6.5% 🟢	+18.8% 🟢	±0.0%
g.s.GraphQLAppliedDirective	-1.3% 🔴	±0.0%	-2.1% 🔴
g.s.GraphQLAppliedDirectiveArgument	-0.9% 🔴	±0.0%	-2.0% 🔴
g.s.GraphQLDirective	-0.2% 🔴	±0.0%	-0.9% 🔴
g.s.GraphQLEnumType	-0.3% 🔴	±0.0%	-1.0% 🔴
g.s.GraphQLEnumValueDefinition	-0.2% 🔴	±0.0%	-0.6% 🔴
g.s.GraphQLFieldDefinition	-0.1% 🔴	±0.0%	-0.2% 🔴
g.s.GraphQLImplementingType	-0.4% 🔴	±0.0%	-1.0% 🔴
g.s.GraphQLInputFieldsContainer	-1.1% 🔴	-10.0% 🔴	-2.5% 🔴
g.s.GraphQLInputObjectField	-0.1% 🔴	±0.0%	-0.2% 🔴
g.s.GraphQLInputObjectType	-0.2% 🔴	±0.0%	-0.6% 🔴
g.s.GraphQLList	-0.8% 🔴	±0.0%	-1.2% 🔴
g.s.GraphQLModifiedType	-0.3% 🔴	±0.0%	-0.7% 🔴
g.s.GraphQLNamedType	+9.5% 🟢	-60.0% 🔴	±0.0%
g.s.GraphQLNonNull	-0.6% 🔴	±0.0%	-0.9% 🔴
g.s.GraphQLUnionType	-0.1% 🔴	±0.0%	-0.5% 🔴
g.s.GraphQLUnmodifiedType	-0.2% 🔴	±0.0%	-0.4% 🔴
g.s.i.e.BaseError	+14.1% 🟢	±0.0%	+16.7% 🟢
g.s.i.TypeInfo	-1.2% 🔴	+2.6% 🟢	-1.4% 🔴
g.s.PropertyFetchingImpl $CacheKey	+11.8% 🟢	-50.0% 🔴	±0.0%
g.s.PropertyFetchingImpl $MethodFinder	-0.3% 🔴	±0.0%	-0.9% 🔴
g.s.v.SchemaValidationError	-6.1% 🔴	-50.0% 🔴	-8.3% 🔴
g.TypeMismatchError	+14.3% 🟢	±0.0%	+22.2% 🟢
g.UnresolvedTypeError	+9.7% 🟢	±0.0%	+15.0% 🟢
g.u.Breadcrumb	+31.4% 🟢	±0.0%	+25.0% 🟢
g.u.NodeLocation	+24.6% 🟢	-50.0% 🔴	+8.3% 🟢
g.v.OperationValidator $FieldAndType	+6.4% 🟢	-50.0% 🔴	-25.0% 🔴
g.VisibleForTesting	±0.0%	-85.7% 🔴	±0.0%

Full HTML report: build artifact jacoco-html-report

Updated: 2026-03-06 12:30:21 UTC

Add jcstress tests for CompletionStageSubscriber race condition verification and integrate them into both PR and master CI workflows. Since jcstress doesn't produce JUnit XML, parse its console output to extract test counts for the unified test report. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tee opens the output file immediately at startup. Since build/ doesn't exist yet on a fresh runner, tee fails to open the file and exits with code 1 (while still copying stdin to stdout). pipefail then picks up that non-zero exit code, failing the step despite gradle succeeding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The test subscribes 3 subscribers to the same publisher and asserts all receive elements in the same order. With a new single-thread executor per mapping call, each CF ran on its own thread, making delivery order depend on thread scheduling. Sometimes elements arrived in order (test passed), sometimes not (assertion failed, caught by TCK, converted to skip). Use a shared single-thread executor per publisher (matching the pattern already used by the Ordered publisher variants) so CFs complete sequentially in submission order. The test now deterministically passes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The Reactive Streams TCK silently converts optional test failures to skips. Add a post-run verification step that asserts exactly 49 tests are skipped, catching any newly-silenced failure or newly-passing test immediately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The previous gate only checked overall line/branch/method coverage, which allowed per-class regressions to pass when offset by improvements elsewhere. Now fails on any class regressing any metric by >0.05%. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Skip classes with 0 total lines (interfaces, annotations, abstract classes) in the per-class coverage gate and changed-class display, fixing 113 false regressions caused by JaCoCo non-determinism for non-concrete classes. Also exclude these from future baseline generation. Add tests for GraphQLList.equals()/hashCode() and for CompletionStageOrderedSubscriber's cfExceptionUnwrap error path (catch block in emptyInFlightQueueIfWeCan). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…notation These methods delegate to super and their coverage is non-deterministic (depends on hash collisions), causing false regressions in the per-class coverage gate. A Gradle task uses ASM to inject an invisible @generated annotation post-compilation, which JaCoCo's AnnotationGeneratedFilter recognizes and excludes from reports. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CI installs JDK 25 and labels the default test run as "java25", but the Gradle toolchain was pinned to 21 causing Gradle to auto-provision JDK 21 instead of using the CI-installed JDK 25. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

andimarek and others added 6 commits March 6, 2026 16:09

andimarek changed the title Fix race condition in CompletionStageSubscriber.onComplete() Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate Mar 6, 2026

andimarek and others added 3 commits March 6, 2026 20:59

Rename test-summary check to Test Report & Per-Class Coverage Gate

460d365

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Bump Java toolchain from 21 to 25 to match CI

099b330

CI installs JDK 25 and labels the default test run as "java25", but the Gradle toolchain was pinned to 21 causing Gradle to auto-provision JDK 21 instead of using the CI-installed JDK 25. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

andimarek merged commit ec31e0b into master Mar 6, 2026
9 of 11 checks passed

Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate by andimarek · Pull Request #4296 · graphql-java/graphql-java · GitHub

Uh oh!

Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate - #4296

Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate#4296
andimarek merged 10 commits into
masterfrom
fix/completion-stage-subscriber-race-condition

andimarek commented Mar 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

github-actions Bot commented Mar 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate by andimarek · Pull Request #4296 · graphql-java/graphql-java · GitHub

Uh oh!

Fix CompletionStageSubscriber race condition, add jcstress tests and CI coverage gate - #4296

Conversation

andimarek commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Race condition fix

jcstress integration

Coverage gate

Missing test coverage

Test plan

Uh oh!

github-actions Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Report

Test Results

Code Coverage (Java 25)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

andimarek commented Mar 6, 2026 •
edited
Loading

github-actions Bot commented Mar 6, 2026 •
edited
Loading