Repository navigation
Fix NullPointerException from activating a null span in scope manager (quick fix) - #12451
Conversation
A null span passed to activateSpan() (e.g. from TracingSendHandler when the websocket span was concurrently cleared by HandlerContext.reset()) was silently pushed as a scope with a null Context. The corrupted scope only surfaced as an NPE on the *next* activation on that thread, when `top.context.with(span)` dereferenced the null context. The existing `assert span != null` never caught this since assertions are never enabled (-ea) in production. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
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. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
More details
Null-span activation now leaves the scope stack untouched, preventing the poisoned context that caused subsequent activations to fail. The websocket callback’s error and cleanup paths remain compatible with a missing span.
🤖 Bits Code Review · Commit 9624bfc · @DataDog review to ask questions
Per review: state the null case explicitly instead of relying on try-with-resources skipping a null resource. Behavior is unchanged, since onError and onFrameEnd both no-op without a span. Also port the null-span scope manager test to ContextScope; AgentScope was removed on master (#12557), so the test no longer compiled after the merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
More details
Null activation now returns before touching the scope stack, preventing corruption of subsequent activations. The websocket fallback preserves callback delivery when its span has been cleared.
🤖 Bits Code Review · Commit 51e8624
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
The merge request has been interrupted because the build 5666697513913241170 took longer than expected. The current limit for the base branch 'master' is 120 minutes. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
ContinuableScopeManager.activatenow returns the existingINVALID_SCOPE(noop) sentinel when given anullspan, instead of silently pushing aContinuableScopewith anullContextonto the scope stack.TracingSendHandler.onResult(JSR-356 websocket async send instrumentation) skipsactivateSpanentirely when the websocket span isnull, matching the existing null-check already used inTracingOutputStream.close()for the sameHandlerContext.ScopeManagerForkedTest#activatingNullSpanReturnsNoopScopeAndDoesNotCorruptStack, which reproduces the original bug: activating anullspan returns a noop scope, and a subsequent real activation on the same thread no longer NPEs.Motivation
Fixes a production
NullPointerExceptionsurfaced via Datadog error tracking (issue6d4f45fa-b461-11f0-8716-da7ad0900002) inContinuableScopeManager.activate.Root cause:
HandlerContext.getWebsocketSpan()can race withHandlerContext.reset()clearing that field from another thread, soTracingSendHandler.onResultcould callactivateSpanwith anullspan.ContinuableScopeManager.activatehad no guard for this — it pushed aContinuableScopewith anullContext. The corruption didn't throw immediately; it only surfaced as an NPE on the next activation on that thread, whentop.context.with(span)dereferenced the poisonednullcontext. The pre-existingassert span != nullnever caught this in practice, since assertions are never enabled (-ea) in production JVMs.Additional Notes
./gradlew :dd-trace-core:forkedTest --tests "datadog.trace.core.scopemanager.ScopeManagerForkedTest"passes../gradlew spotlessApplyproduces no additional changes.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueJira ticket: N/A — originated from Datadog Error Tracking issue
6d4f45fa-b461-11f0-8716-da7ad0900002🤖 Generated with Claude Code