Skip to content

Commit e32291a

Browse files
PerfectSlayerbric3
andauthored
🍒 11111 - Preload scope classes to prevent virtual thread deadlock (#11131)
* feat(agent): Move forced class preload into instrumenter module (cherry picked from commit e95e91e) * fix(virtual-thread): Preload scope classes to prevent virtual thread deadlock Preload ScopeContext and ScopeStack classes in ContinuableScopeManager constructor to avoid class loading on virtual thread mount path. DatadogClassLoader's synchronized I/O from JarFile can pin carrier threads and deadlock the application. (cherry picked from commit c521b50) * fix(virtual-thread): Add support for early VirtualThread afterTerminate method Support both afterDone() and afterTerminate() cleanup methods to handle different JDK 21 virtual thread implementations. Both methods cancel the help continuation and release the context scope when the virtual thread completes. (cherry picked from commit d223f79) --------- Co-authored-by: Brice Dutheil <brice.dutheil@gmail.com>
1 parent ced8dd1 commit e32291a

File tree

3 files changed

+44
-34
lines changed

3 files changed

+44
-34
lines changed

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/InstrumenterModule.java

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public int order() {
9090
}
9191

9292
public List<Instrumenter> typeInstrumentations() {
93+
preloadClasses();
9394
return singletonList(this);
9495
}
9596

@@ -214,6 +215,28 @@ protected final boolean isShortcutMatchingEnabled(boolean defaultToShortcut) {
214215
.isIntegrationShortcutMatchingEnabled(singletonList(name()), defaultToShortcut);
215216
}
216217

218+
/**
219+
* Force loading of classes that need to be instrumented, but are using during instrumentation.
220+
*/
221+
@SuppressForbidden // allow this use of Class.forName()
222+
protected void preloadClasses() {
223+
String[] list = preloadClassNames();
224+
if (list != null) {
225+
for (String clazz : list) {
226+
try {
227+
Class.forName(clazz);
228+
} catch (Throwable t) {
229+
log.debug("Error force loading {} class", clazz);
230+
}
231+
}
232+
}
233+
}
234+
235+
/** Get classes to force load */
236+
public String[] preloadClassNames() {
237+
return null;
238+
}
239+
217240
/** Parent class for all tracing related instrumentations */
218241
public abstract static class Tracing extends InstrumenterModule {
219242
public Tracing(String instrumentationName, String... additionalNames) {
@@ -258,18 +281,11 @@ public final boolean isApplicable(Set<TargetSystem> enabledSystems) {
258281
}
259282

260283
/** Parent class for all IAST related instrumentations */
261-
@SuppressForbidden
262284
public abstract static class Iast extends InstrumenterModule {
263285
public Iast(String instrumentationName, String... additionalNames) {
264286
super(instrumentationName, additionalNames);
265287
}
266288

267-
@Override
268-
public List<Instrumenter> typeInstrumentations() {
269-
preloadClassNames();
270-
return super.typeInstrumentations();
271-
}
272-
273289
@Override
274290
public final boolean isApplicable(Set<TargetSystem> enabledSystems) {
275291
if (enabledSystems.contains(TargetSystem.IAST)) {
@@ -282,27 +298,6 @@ public final boolean isApplicable(Set<TargetSystem> enabledSystems) {
282298
return cfg.getAppSecActivation() == ProductActivation.FULLY_ENABLED;
283299
}
284300

285-
/**
286-
* Force loading of classes that need to be instrumented, but are using during instrumentation.
287-
*/
288-
private void preloadClassNames() {
289-
String[] list = getClassNamesToBePreloaded();
290-
if (list != null) {
291-
for (String clazz : list) {
292-
try {
293-
Class.forName(clazz);
294-
} catch (Throwable t) {
295-
log.debug("Error force loading {} class", clazz);
296-
}
297-
}
298-
}
299-
}
300-
301-
/** Get classes to force load* */
302-
public String[] getClassNamesToBePreloaded() {
303-
return null;
304-
}
305-
306301
@Override
307302
public Advice.PostProcessor.Factory postProcessor() {
308303
return IastPostProcessorFactory.INSTANCE;

dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/InputStreamInstrumentation.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
public class InputStreamInstrumentation extends InstrumenterModule.Iast
2121
implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
2222

23-
private static final String[] FORCE_LOADING = {"java.io.PushbackInputStream"};
23+
private static final String[] PRELOAD_CLASS_NAMES = {"java.io.PushbackInputStream"};
2424

2525
public InputStreamInstrumentation() {
2626
super("inputStream");
@@ -44,8 +44,8 @@ public void methodAdvice(MethodTransformer transformer) {
4444
}
4545

4646
@Override
47-
public String[] getClassNamesToBePreloaded() {
48-
return FORCE_LOADING;
47+
public String[] preloadClassNames() {
48+
return PRELOAD_CLASS_NAMES;
4949
}
5050

5151
public static class InputStreamAdvice {

dd-java-agent/instrumentation/java/java-lang/java-lang-21.0/src/main/java/datadog/trace/instrumentation/java/lang/jdk21/VirtualThreadInstrumentation.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@
4545
* <li>{@code unmount()}: swaps the carrier thread's original context back, saving the virtual
4646
* thread's (possibly modified) context for the next mount.
4747
* <li>Steps 2-3 repeat on each park/unpark cycle, potentially on different carrier threads.
48-
* <li>{@code afterDone()}: cancels the help continuation, releasing the context scope to be
49-
* closed.
48+
* <li>{@code afterDone()} / {@code afterTerminate()} for early VirtualThread support: cancels the
49+
* help continuation, releasing the context scope to be closed.
5050
* </ol>
5151
*
5252
* <p>Instrumenting the internal {@code VirtualThread.runContinuation()} method does not work as the
@@ -66,10 +66,22 @@ public final class VirtualThreadInstrumentation extends InstrumenterModule.Conte
6666
Instrumenter.HasMethodAdvice,
6767
ExcludeFilterProvider {
6868

69+
// Preload classes used by Context.swap() to avoid class loading on the virtual thread mount path.
70+
// DatadogClassLoader loads these from a JarFile using synchronized I/O, which pins
71+
// virtual thread carrier threads and can deadlock the application.
72+
private static final String[] PRELOAD_CLASS_NAMES = {
73+
"datadog.trace.core.scopemanager.ScopeContext", "datadog.trace.core.scopemanager.ScopeStack"
74+
};
75+
6976
public VirtualThreadInstrumentation() {
7077
super("java-lang", "java-lang-21", "virtual-thread");
7178
}
7279

80+
@Override
81+
public String[] preloadClassNames() {
82+
return PRELOAD_CLASS_NAMES;
83+
}
84+
7385
@Override
7486
public String instrumentedType() {
7587
return VIRTUAL_THREAD_CLASS_NAME;
@@ -99,6 +111,9 @@ public void methodAdvice(MethodTransformer transformer) {
99111
transformer.applyAdvice(
100112
isMethod().and(named("afterDone")).and(takesArguments(boolean.class)),
101113
getClass().getName() + "$AfterDone");
114+
transformer.applyAdvice(
115+
isMethod().and(named("afterTerminate")).and(takesArguments(boolean.class, boolean.class)),
116+
getClass().getName() + "$AfterDone");
102117
}
103118

104119
public static final class Construct {
@@ -141,7 +156,7 @@ public static void onUnmount(@Advice.This Object virtualThread) {
141156

142157
public static final class AfterDone {
143158
@OnMethodEnter(suppress = Throwable.class)
144-
public static void onTerminate(@Advice.This Object virtualThread) {
159+
public static void onDone(@Advice.This Object virtualThread) {
145160
ContextStore<Object, VirtualThreadState> store =
146161
InstrumentationContext.get(VIRTUAL_THREAD_CLASS_NAME, VIRTUAL_THREAD_STATE_CLASS_NAME);
147162
VirtualThreadState state = store.remove(virtualThread);

0 commit comments

Comments
 (0)