Skip to content

Commit 02d8d51

Browse files
committed
Remove CompletableFuture from TempLocationManager
(cherry picked from commit d0eed45)
1 parent badff4f commit 02d8d51

File tree

1 file changed

+40
-25
lines changed
  • dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller

1 file changed

+40
-25
lines changed

dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/TempLocationManager.java

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import datadog.trace.api.config.ProfilingConfig;
44
import datadog.trace.bootstrap.config.provider.ConfigProvider;
5+
import datadog.trace.util.AgentTaskScheduler;
56
import datadog.trace.util.PidHelper;
67
import java.io.IOException;
78
import java.nio.file.FileVisitResult;
@@ -15,10 +16,8 @@
1516
import java.time.Instant;
1617
import java.time.temporal.ChronoUnit;
1718
import java.util.Set;
18-
import java.util.concurrent.CompletableFuture;
19-
import java.util.concurrent.ExecutionException;
19+
import java.util.concurrent.CountDownLatch;
2020
import java.util.concurrent.TimeUnit;
21-
import java.util.concurrent.TimeoutException;
2221
import org.slf4j.Logger;
2322
import org.slf4j.LoggerFactory;
2423

@@ -62,7 +61,7 @@ default FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOE
6261
default void onCleanupStart(boolean selfCleanup, long timeout, TimeUnit unit) {}
6362
}
6463

65-
private class CleanupVisitor implements FileVisitor<Path> {
64+
private final class CleanupVisitor implements FileVisitor<Path> {
6665
private boolean shouldClean;
6766

6867
private final Set<String> pidSet = PidHelper.getJavaPids();
@@ -173,12 +172,37 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
173172
}
174173
}
175174

175+
private final class CleanupTask implements Runnable {
176+
private final CountDownLatch latch = new CountDownLatch(1);
177+
private volatile Throwable throwable = null;
178+
179+
@Override
180+
public void run() {
181+
try {
182+
cleanup(false);
183+
} catch (OutOfMemoryError oom) {
184+
throw oom;
185+
} catch (Throwable t) {
186+
throwable = t;
187+
} finally {
188+
latch.countDown();
189+
}
190+
}
191+
192+
boolean await(long timeout, TimeUnit unit) throws Throwable {
193+
boolean ret = latch.await(timeout, unit);
194+
if (throwable != null) {
195+
throw throwable;
196+
}
197+
return ret;
198+
}
199+
}
200+
176201
private final Path baseTempDir;
177202
private final Path tempDir;
178203
private final long cutoffSeconds;
179204

180-
private final CompletableFuture<Void> cleanupTask;
181-
205+
private final CleanupTask cleanupTask = new CleanupTask();
182206
private final CleanupHook cleanupTestHook;
183207

184208
/**
@@ -200,11 +224,7 @@ public static TempLocationManager getInstance() {
200224
static TempLocationManager getInstance(boolean waitForCleanup) {
201225
TempLocationManager instance = SingletonHolder.INSTANCE;
202226
if (waitForCleanup) {
203-
try {
204-
instance.waitForCleanup(5, TimeUnit.SECONDS);
205-
} catch (TimeoutException ignored) {
206-
207-
}
227+
instance.waitForCleanup(5, TimeUnit.SECONDS);
208228
}
209229
return instance;
210230
}
@@ -256,20 +276,17 @@ private TempLocationManager() {
256276
baseTempDir.toFile().deleteOnExit();
257277

258278
tempDir = baseTempDir.resolve("pid_" + pid);
259-
cleanupTask = CompletableFuture.runAsync(() -> cleanup(false));
279+
AgentTaskScheduler.INSTANCE.execute(() -> cleanup(false));
260280

261281
Thread selfCleanup =
262282
new Thread(
263283
() -> {
264-
try {
265-
waitForCleanup(1, TimeUnit.SECONDS);
266-
} catch (TimeoutException e) {
284+
if (!waitForCleanup(1, TimeUnit.SECONDS)) {
267285
log.info(
268286
"Cleanup task timed out. {} temp directory might not have been cleaned up properly",
269287
tempDir);
270-
} finally {
271-
cleanup(true);
272288
}
289+
cleanup(true);
273290
},
274291
"Temp Location Manager Cleanup");
275292
Runtime.getRuntime().addShutdownHook(selfCleanup);
@@ -359,21 +376,19 @@ boolean cleanup(boolean cleanSelf, long timeout, TimeUnit unit) {
359376
}
360377

361378
// accessible for tests
362-
void waitForCleanup(long timeout, TimeUnit unit) throws TimeoutException {
379+
boolean waitForCleanup(long timeout, TimeUnit unit) {
363380
try {
364-
cleanupTask.get(timeout, unit);
381+
return cleanupTask.await(timeout, unit);
365382
} catch (InterruptedException e) {
366-
cleanupTask.cancel(true);
383+
log.debug("Temp directory cleanup was interrupted");
367384
Thread.currentThread().interrupt();
368-
} catch (TimeoutException e) {
369-
cleanupTask.cancel(true);
370-
throw e;
371-
} catch (ExecutionException e) {
385+
} catch (Throwable t) {
372386
if (log.isDebugEnabled()) {
373-
log.debug("Failed to cleanup temp directory: {}", tempDir, e);
387+
log.debug("Failed to cleanup temp directory: {}", tempDir, t);
374388
} else {
375389
log.debug("Failed to cleanup temp directory: {}", tempDir);
376390
}
377391
}
392+
return false;
378393
}
379394
}

0 commit comments

Comments
 (0)