Skip to content

Commit cb71f86

Browse files
authored
Avoid leaking continuations on jdk-http-client (#11557)
Avoid leaking continuations on jdk-http-client remove matchers for send Co-authored-by: andrea.marziali <andrea.marziali@datadoghq.com>
1 parent 44e5f0a commit cb71f86

4 files changed

Lines changed: 19 additions & 13 deletions

File tree

dd-java-agent/instrumentation/java/java-concurrent/java-concurrent-1.8/src/main/java/datadog/trace/instrumentation/java/concurrent/AsyncPropagatingDisableInstrumentation.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public AsyncPropagatingDisableInstrumentation() {
4747
namedOneOf("reactor.core.scheduler.SchedulerTask", "reactor.core.scheduler.WorkerTask");
4848
private static final ElementMatcher<TypeDescription> RXJAVA2_DISABLED_TYPE_INITIALIZERS =
4949
named("io.reactivex.internal.schedulers.AbstractDirectTask");
50+
private static final ElementMatcher<TypeDescription> JAVA_HTTP_CLIENT =
51+
extendsClass(named("java.net.http.HttpClient"));
5052

5153
@Override
5254
public boolean onlyMatchKnownTypes() {
@@ -80,7 +82,8 @@ public String[] knownMatchingTypes() {
8082
"org.springframework.jms.listener.DefaultMessageListenerContainer",
8183
"org.apache.activemq.broker.TransactionBroker",
8284
"com.mongodb.internal.connection.DefaultConnectionPool$AsyncWorkManager",
83-
"io.reactivex.internal.schedulers.AbstractDirectTask"
85+
"io.reactivex.internal.schedulers.AbstractDirectTask",
86+
"jdk.internal.net.http.HttpClientImpl"
8487
};
8588
}
8689

@@ -94,7 +97,8 @@ public ElementMatcher<TypeDescription> hierarchyMatcher() {
9497
return RX_WORKERS
9598
.or(GRPC_MANAGED_CHANNEL)
9699
.or(REACTOR_DISABLED_TYPE_INITIALIZERS)
97-
.or(RXJAVA2_DISABLED_TYPE_INITIALIZERS);
100+
.or(RXJAVA2_DISABLED_TYPE_INITIALIZERS)
101+
.or(JAVA_HTTP_CLIENT);
98102
}
99103

100104
@Override
@@ -180,6 +184,7 @@ public void methodAdvice(MethodTransformer transformer) {
180184
isTypeInitializer().and(isDeclaredBy(REACTOR_DISABLED_TYPE_INITIALIZERS)), advice);
181185
transformer.applyAdvice(
182186
isTypeInitializer().and(isDeclaredBy(RXJAVA2_DISABLED_TYPE_INITIALIZERS)), advice);
187+
transformer.applyAdvice(namedOneOf("sendAsync").and(isDeclaredBy(JAVA_HTTP_CLIENT)), advice);
183188
}
184189

185190
public static class DisableAsyncAdvice {

dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java11/datadog/trace/instrumentation/httpclient/BodyHandlerWrapper.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package datadog.trace.instrumentation.httpclient;
22

3+
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.captureSpan;
4+
35
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
6+
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
47
import java.net.http.HttpResponse.BodyHandler;
58
import java.net.http.HttpResponse.BodySubscriber;
69
import java.net.http.HttpResponse.ResponseInfo;
@@ -11,20 +14,21 @@
1114

1215
public class BodyHandlerWrapper<T> implements BodyHandler<T> {
1316
private final BodyHandler<T> delegate;
14-
private final AgentScope.Continuation continuation;
17+
private final AgentSpan span;
1518

16-
public BodyHandlerWrapper(BodyHandler<T> delegate, AgentScope.Continuation context) {
19+
public BodyHandlerWrapper(BodyHandler<T> delegate, AgentSpan span) {
1720
this.delegate = delegate;
18-
this.continuation = context;
21+
this.span = span;
1922
}
2023

2124
@Override
2225
public BodySubscriber<T> apply(ResponseInfo responseInfo) {
26+
// Capture the continuation lazily here rather than at sendAsync() call time.
2327
BodySubscriber<T> subscriber = delegate.apply(responseInfo);
2428
if (subscriber instanceof BodySubscriberWrapper) {
2529
return subscriber;
2630
}
27-
return new BodySubscriberWrapper<>(subscriber, continuation);
31+
return new BodySubscriberWrapper<>(subscriber, captureSpan(span));
2832
}
2933

3034
static class BodySubscriberWrapper<T> implements BodySubscriber<T> {

dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/main/java11/datadog/trace/instrumentation/httpclient/SendAsyncAdvice.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package datadog.trace.instrumentation.httpclient;
22

33
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
4-
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.captureSpan;
54
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
65
import static datadog.trace.instrumentation.httpclient.JavaNetClientDecorator.DECORATE;
76
import static datadog.trace.instrumentation.httpclient.JavaNetClientDecorator.INSTRUMENTATION_NAME;
@@ -38,7 +37,10 @@ public static AgentScope methodEnter(
3837
final AgentSpan span = startSpan(INSTRUMENTATION_NAME, OPERATION_NAME);
3938
final AgentScope scope = activateSpan(span);
4039
if (bodyHandler != null) {
41-
bodyHandler = new BodyHandlerWrapper<>(bodyHandler, captureSpan(span));
40+
// Pass span directly — BodyHandlerWrapper captures the continuation lazily in apply(),
41+
// only once response headers arrive. This avoids leaking a continuation when the
42+
// connection fails before headers are received.
43+
bodyHandler = new BodyHandlerWrapper<>(bodyHandler, span);
4244
}
4345

4446
DECORATE.afterStart(span);

dd-java-agent/instrumentation/java/java-net/java-net-11.0/src/test/groovy/datadog/trace/instrumentation/httpclient/JavaHttpClientTest.groovy

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ import java.net.http.HttpResponse
1010
import java.time.Duration
1111

1212
abstract class JavaHttpClientTest extends HttpClientTest {
13-
@Override
14-
boolean useStrictTraceWrites() {
15-
// TODO fix this by making sure that spans get closed properly
16-
return false
17-
}
1813

1914
def client = HttpClient.newBuilder()
2015
.connectTimeout(Duration.ofMillis(CONNECT_TIMEOUT_MS))

0 commit comments

Comments
 (0)