Fix memory leak in WeakMapSuppliers.WeakConcurrent
The problem there is that it references JVM shutdown hook that keeps
reference to cleanup executor which potentially can keep references to
all sort of things - and this doesn't get cleaned up untill JVM
shutdown.
Solution is to remove shutdown hook when supplier is being GCed.
MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java
@@ -29,10 +29,6 @@ public interface WeakMap<K, V> {
}
}
- public static boolean isProviderRegistered() {
- return provider.get() != Supplier.DEFAULT;
- }
-
public static <K, V> WeakMap<K, V> newWeakMap() {
return provider.get().get();
}

MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/AgentInstaller.java
@@ -111,9 +111,7 @@ public class AgentInstaller {
}
private static void registerWeakMapProvider() {
- if (!WeakMap.Provider.isProviderRegistered()) {
WeakMap.Provider.registerIfAbsent(new WeakMapSuppliers.WeakConcurrent());
- }
// WeakMap.Provider.registerIfAbsent(new WeakMapSuppliers.WeakConcurrent.Inline());
// WeakMap.Provider.registerIfAbsent(new WeakMapSuppliers.Guava());
}

MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/WeakMapSuppliers.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/WeakMapSuppliers.java
@@ -54,7 +54,7 @@ class WeakMapSuppliers {
};
private final ScheduledExecutorService cleanerExecutorService;
-
+ private final Thread shutdownCallback;
private final Queue<WeakReference<WeakConcurrentMap>> suppliedMaps =
new ConcurrentLinkedQueue<>();
@@ -62,14 +62,16 @@ class WeakMapSuppliers {
WeakConcurrent() {
cleanerExecutorService = Executors.newScheduledThreadPool(1, THREAD_FACTORY);
+ shutdownCallback = new ShutdownCallback(cleanerExecutorService);
+
cleanerExecutorService.scheduleAtFixedRate(
- new CleanupRunnable(cleanerExecutorService, suppliedMaps, finalized),
+ new CleanupRunnable(cleanerExecutorService, shutdownCallback, suppliedMaps, finalized),
CLEAN_FREQUENCY_SECONDS,
CLEAN_FREQUENCY_SECONDS,
TimeUnit.SECONDS);
try {
- Runtime.getRuntime().addShutdownHook(new ShutdownCallback(cleanerExecutorService));
+ Runtime.getRuntime().addShutdownHook(shutdownCallback);
} catch (final IllegalStateException ex) {
// The JVM is already shutting down.
}
@@ -90,14 +92,17 @@ class WeakMapSuppliers {
private static class CleanupRunnable implements Runnable {
private final ScheduledExecutorService executorService;
+ private final Thread shutdownCallback;
private final Queue<WeakReference<WeakConcurrentMap>> suppliedMaps;
private final AtomicBoolean finalized;
public CleanupRunnable(
final ScheduledExecutorService executorService,
+ final Thread shutdownCallback,
final Queue<WeakReference<WeakConcurrentMap>> suppliedMaps,
final AtomicBoolean finalized) {
this.executorService = executorService;
+ this.shutdownCallback = shutdownCallback;
this.suppliedMaps = suppliedMaps;
this.finalized = finalized;
}
@@ -115,6 +120,7 @@ class WeakMapSuppliers {
}
if (finalized.get() && suppliedMaps.isEmpty()) {
executorService.shutdown();
+ Runtime.getRuntime().removeShutdownHook(shutdownCallback);
}
}
}

MODIFY
dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakConcurrentSupplierTest.groovy
dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakConcurrentSupplierTest.groovy
@@ -107,7 +107,7 @@ class WeakConcurrentSupplierTest extends Specification {
// Hit map a few times to trigger unreferenced entries cleanup.
// Exact number of times that we need to hit map is implementation dependent.
- // For Guava it i specified in
+ // For Guava it is specified in
// com.google.common.collect.MapMakerInternalMap.DRAIN_THRESHOLD = 0x3F
if (name == "Guava" || name == "WeakInline") {
for (int i = 0; i <= 0x3F; i++) {

Fix memory leak in WeakMapSuppliers.WeakConcurrent

The problem there is that it references JVM shutdown hook that keeps
reference to cleanup executor which potentially can keep references to
all sort of things - and this doesn't get cleaned up untill JVM
shutdown.

Solution is to remove shutdown hook when supplier is being GCed.