Repository navigation
Conversation
|
@DataDog review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
On kotlinx-coroutines 1.8, SelectBuilderImpl no longer implements Continuation, so Byte Buddy rejects the typed @Advice.This binding at transformation time before any runtime suppression can apply. The fix is to bind the receiver as Object and add a Continuation instanceof guard, letting modern select builders delegate to the already-instrumented CancellableContinuationImpl.
🤖 Bits Code Review · Commit a9bcd5c · @DataDog review to ask questions
| public static void beforeSuspension(@Advice.This Continuation<?> continuation) { | ||
| DatadogThreadContextElement.beforeSuspension(continuation); |
There was a problem hiding this comment.
When legacy select code runs with supported kotlinx-coroutines 1.8, SelectBuilderImpl.getResult() matches this advice, but its receiver no longer implements Continuation. Byte Buddy rejects the transformation before suppress=Throwable can apply, producing an instrumentation error. An Object receiver with a Continuation guard preserves older-version behavior and avoids the rejection; the newer builder delegates suspension to the already-instrumented CancellableContinuationImpl.getResult().
| public static void beforeSuspension(@Advice.This Continuation<?> continuation) { | |
| DatadogThreadContextElement.beforeSuspension(continuation); | |
| public static void beforeSuspension(@Advice.This Object receiver) { | |
| if (receiver instanceof Continuation) { | |
| DatadogThreadContextElement.beforeSuspension((Continuation<?>) receiver); | |
| } |
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · @DataDog review to ask questions · Open Bits AI session
🟢 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. |
| * Snapshots context before Kotlin publishes standard suspension or yield decisions. Custom | ||
| * low-level suspension primitives can bypass these hooks. | ||
| */ | ||
| public class SuspensionInstrumentation |
There was a problem hiding this comment.
We could consider only enabling this if InstrumenterConfig.get().isLegacyContextManagerEnabled()
| } | ||
| } | ||
|
|
||
| static final class State { |
There was a problem hiding this comment.
Would Exchange read better? (avoids confusion with the other State class)
What Does This Do
A Kotlin coroutine can resume on another worker before the previous worker restores its thread context, losing a manually opened scope and leaving it unclosed.
Capture the scope stack before standard suspension and yield decisions, and prevent an older restoration from overwriting newer context. Enable instrumentation of
SafeContinuationwith a narrow allow-list exception.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]