Fix context propagation when the same Runnable is submitted to a ThreadPoolExecutor multiple times - #12379
Fix context propagation when the same Runnable is submitted to a ThreadPoolExecutor multiple times#12379amarziali wants to merge 1 commit into
Conversation
…adPoolExecutor multiple times
|
@DataDog review |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 537bbdd | Docs | View more details | Give us feedback! |
![datadog-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The ownership logic can cancel the wrong context and can miss cleanup when queue admission throws. Manual removal can block executor termination, and rejection can inflate cancellation metrics.
🤖 Datadog Autotest · Commit 537bbdd · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| } | ||
| owner = queued; | ||
| State state = contextStore.get(queued); | ||
| continuation = state == null ? null : state.getCancellableContinuation(); |
There was a problem hiding this comment.
The remaining traced submission runs without its parent context.
Assertion details
- Input: Queue one reusable task with root context. Queue the same task with traced context. Then remove one task.
- Expected:
Removal releases only the continuation that belongs to the removed queue entry. - Actual:
The scan reads the later traced continuation from task-wide state. It then releases that continuation after it removes the earlier root-context entry.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
| if (queued instanceof Wrapper) { | ||
| if (tpe.getQueue().remove(queued)) { | ||
| ((Wrapper<?>) queued).cancel(); | ||
| return true; |
There was a problem hiding this comment.
The executor can stay in shutdown state, and awaitTermination can wait forever.
Assertion details
- Input: Shut down an executor that has no workers and one wrapped task. Then remove that task by its original identity.
- Expected:
The executor checks for termination after it removes the final queued task. - Actual:
The advice removes the wrapper and skips the executor method body. It does not call the executor termination check.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
| InstrumentationContext.get(Runnable.class, State.class), | ||
| task, | ||
| currentContext(), | ||
| tpe); |
There was a problem hiding this comment.
The continuation can keep a trace open and can later give reused work the wrong parent context.
Assertion details
- Input: Submit a traced task to a queue whose offer operation throws, such as an incompatible task in a populated PriorityBlockingQueue.
- Expected:
The executor releases the owned continuation when queue admission throws. - Actual:
The enter advice installs a tagged continuation. Generic exception cleanup ignores this continuation, and this advice has no exception cleanup.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
| if (wrapper != null) { | ||
| wrapper.cancel(); | ||
| } else if (continuation != null) { | ||
| continuation.release(); |
There was a problem hiding this comment.
Executor saturation inflates continuation cancellation health metrics.
Assertion details
- Input: Reject any propagated task through an instrumented rejection handler.
- Expected:
Rejection ends each continuation once and records one finished continuation. - Actual:
Scope close finishes the resumed continuation. The following explicit release records a second cancellation.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
1 participant
What Does This Do
Fix context propagation when the same Runnable instance is submitted concurrently, while preserving the original runnable identity. Each submission now owns its continuation independently, preventing missing or incorrectly parented traces.
The change preserves ThreadPoolExecutor subclass compatibility, handles rejection/removal/shutdown cleanup, and inspects queue compatibility only when a collision occurs.
Benchmark
JMH on JDK 25/macOS found:
Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]