Wrap finalizers into try/catch
MODIFY
dd-trace/src/main/java/datadog/trace/tracer/ContinuationImpl.java
dd-trace/src/main/java/datadog/trace/tracer/ContinuationImpl.java
package datadog.trace.tracer;
+import lombok.extern.slf4j.Slf4j;
+
/** Concrete implementation of a continuation */
+@Slf4j
class ContinuationImpl implements Continuation {
private final TraceInternal trace;
@@ -36,11 +39,9 @@ class ContinuationImpl implements Continuation {
}
}
- // TODO: we may want to reconsider usage of 'finalize'. One of the problems seems to be that
- // exceptions thrown in finalizer are eaten up and ignored, and may not even be logged by default.
- // This may lead to fun debugging sessions.
@Override
protected synchronized void finalize() {
+ try {
if (!closed) {
trace
.getTracer()
@@ -49,6 +50,10 @@ class ContinuationImpl implements Continuation {
this);
closeContinuation(true);
}
+ } catch (final Throwable t) {
+ // Exceptions thrown in finalizer are eaten up and ignored, so log them instead
+ log.debug("Span finalizer had thrown an exception: ", t);
+ }
}
/**

MODIFY
dd-trace/src/main/java/datadog/trace/tracer/SpanImpl.java
dd-trace/src/main/java/datadog/trace/tracer/SpanImpl.java
@@ -6,8 +6,10 @@ import java.io.StringWriter;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import lombok.extern.slf4j.Slf4j;
/** Concrete implementation of a span */
+@Slf4j
class SpanImpl implements Span {
private final TraceInternal trace;
@@ -217,13 +219,12 @@ class SpanImpl implements Span {
}
}
- // TODO: we may want to reconsider usage of 'finalize'. One of the problems seems to be that
- // exceptions thrown in finalizer are eaten up and ignored, and may not even be logged by default.
- // This may lead to fun debugging sessions.
@Override
protected synchronized void finalize() {
- // Note: according to docs finalize is only called once for a given instance - even if instance
- // if 'revived' from the dead by passing reference to some other object and then dies again.
+ try {
+ // Note: according to docs finalize is only called once for a given instance - even if
+ // instance is 'revived' from the dead by passing reference to some other object and
+ // then dies again.
if (!isFinished()) {
trace
.getTracer()
@@ -231,6 +232,10 @@ class SpanImpl implements Span {
"Finishing span due to GC, this will prevent trace from being reported: %s", this);
finishSpan(startTimestamp.getDuration(), true);
}
+ } catch (final Throwable t) {
+ // Exceptions thrown in finalizer are eaten up and ignored, so log them instead
+ log.debug("Span finalizer had thrown an exception: ", t);
+ }
}
/**

MODIFY
dd-trace/src/test/groovy/datadog/trace/tracer/ContinuationImplTest.groovy
dd-trace/src/test/groovy/datadog/trace/tracer/ContinuationImplTest.groovy
@@ -90,4 +90,16 @@ class ContinuationImplTest extends Specification {
0 * tracer.reportError(_, *_)
}
+ def "finalize catches all exceptions"() {
+ setup:
+ def continuation = new ContinuationImpl(trace, span)
+
+ when:
+ continuation.finalize()
+
+ then:
+ 1 * trace.getTracer() >> { throw new Throwable() }
+ noExceptionThrown()
+ }
+
}

MODIFY
dd-trace/src/test/groovy/datadog/trace/tracer/SpanImplTest.groovy
dd-trace/src/test/groovy/datadog/trace/tracer/SpanImplTest.groovy
@@ -270,6 +270,18 @@ class SpanImplTest extends Specification {
0 * tracer.reportError(_, *_)
}
+ def "finalize catches all exceptions"() {
+ setup:
+ def span = new SpanImpl(trace, parentContext, startTimestamp)
+
+ when:
+ span.finalize()
+
+ then:
+ 1 * startTimestamp.getDuration() >> { throw new Throwable() }
+ noExceptionThrown()
+ }
+
def "span without timestamp"() {
when:
new SpanImpl(trace, parentContext, null)

Wrap finalizers into try/catch