Review fixes and add tests.
MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Cleaner.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Cleaner.java
@@ -11,8 +11,6 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
class Cleaner {
- // TODO: add unit tests for this class.
-
private static final long SHUTDOWN_WAIT_SECONDS = 5;
private static final ThreadFactory THREAD_FACTORY =
@@ -47,6 +45,7 @@ class Cleaner {
log.warn("Cleaning scheduled but cleaner is shutdown. Target won't be cleaned {}", target);
} else {
try {
+ // Schedule job and save future to allow job to be canceled if target is GC'd.
command.setFuture(cleanerService.scheduleAtFixedRate(command, frequency, frequency, unit));
} catch (final RejectedExecutionException e) {
log.warn("Cleaning task rejected. Target won't be cleaned {}", target);
@@ -72,7 +71,7 @@ class Cleaner {
private static class CleanupRunnable<T> implements Runnable {
private final WeakReference<T> target;
private final Adapter<T> adapter;
- private ScheduledFuture<?> future = null;
+ private volatile ScheduledFuture<?> future = null;
private CleanupRunnable(final T target, final Adapter<T> adapter) {
this.target = new WeakReference<>(target);

MODIFY
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDCachingPoolStrategy.java
dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/DDCachingPoolStrategy.java
@@ -73,13 +73,11 @@ public class DDCachingPoolStrategy implements PoolStrategy {
* The cache only does cleanup on occasional reads and writes.
* We want to ensure this happens more regularly, so we schedule a thread to do run cleanup manually.
*/
- final long cleanFrequency = expireDuration / 2;
- cleaner.scheduleCleaning(cache, CacheCleaner.CLEANER, cleanFrequency, unit);
+ cleaner.scheduleCleaning(cache, CacheCleaner.CLEANER, expireDuration, unit);
}
private static EvictingCacheProvider withObjectType(
final Cleaner cleaner, final long expireDuration, final TimeUnit unit) {
- assert expireDuration % 2 == 0 : "expireDuration must be even";
final EvictingCacheProvider cacheProvider =
new EvictingCacheProvider(cleaner, expireDuration, unit);
cacheProvider.register(

ADD
dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/CleanerTest.groovy
+package datadog.trace.agent.tooling
+
+import datadog.trace.util.gc.GCUtils
+import spock.lang.Specification
+import spock.lang.Subject
+
+import java.lang.ref.WeakReference
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.atomic.AtomicInteger
+
+import static java.util.concurrent.TimeUnit.MILLISECONDS
+
+class CleanerTest extends Specification {
+
+ @Subject
+ def cleaner = new Cleaner()
+
+ def cleanup() {
+ cleaner.stop()
+ }
+
+ def "test scheduling"() {
+ setup:
+ def latch = new CountDownLatch(2)
+ def target = new Object()
+ def action = new Cleaner.Adapter<Object>() {
+ @Override
+ void clean(Object t) {
+ latch.countDown()
+ }
+ }
+
+ expect:
+ !cleaner.cleanerService.isShutdown()
+
+ when:
+ cleaner.scheduleCleaning(target, action, 10, MILLISECONDS)
+
+ then:
+ latch.await(100, MILLISECONDS)
+ }
+
+ def "test canceling"() {
+ setup:
+ def callCount = new AtomicInteger()
+ def target = new WeakReference(new Object())
+ def action = new Cleaner.Adapter<Object>() {
+ @Override
+ void clean(Object t) {
+ callCount.incrementAndGet()
+ }
+ }
+
+ expect:
+ !cleaner.cleanerService.isShutdown()
+
+ when:
+ cleaner.scheduleCleaning(target.get(), action, 10, MILLISECONDS)
+ GCUtils.awaitGC(target)
+ Thread.sleep(1)
+ def snapshot = callCount.get()
+ Thread.sleep(11)
+
+ then:
+ snapshot == callCount.get()
+ }
+
+ def "test null target"() {
+ setup:
+ def callCount = new AtomicInteger()
+ def action = new Cleaner.Adapter<Object>() {
+ @Override
+ void clean(Object t) {
+ callCount.incrementAndGet()
+ }
+ }
+
+ expect:
+ !cleaner.cleanerService.isShutdown()
+
+ when:
+ cleaner.scheduleCleaning(null, action, 10, MILLISECONDS)
+ Thread.sleep(11)
+
+ then:
+ callCount.get() == 0
+ }
+
+ def "test shutdown"() {
+ expect:
+ !cleaner.cleanerService.isShutdown()
+
+ when:
+ cleaner.stop()
+
+ then:
+ cleaner.cleanerService.awaitTermination(50, MILLISECONDS)
+ cleaner.cleanerService.isTerminated()
+ }
+}

MODIFY
dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/EvictingCacheProviderTest.groovy
dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/EvictingCacheProviderTest.groovy
@@ -4,6 +4,7 @@ import datadog.trace.util.gc.GCUtils
import net.bytebuddy.description.type.TypeDescription
import net.bytebuddy.pool.TypePool
import spock.lang.Specification
+import spock.lang.Timeout
import java.lang.ref.WeakReference
import java.util.concurrent.TimeUnit
@@ -11,7 +12,7 @@ import java.util.concurrent.atomic.AtomicReference
import static datadog.trace.agent.tooling.AgentTooling.CLEANER
-//@Timeout(5)
+@Timeout(5)
class EvictingCacheProviderTest extends Specification {
def "test provider"() {

Review fixes and add tests.