Skip to content

Improve TempLocationManagerTest stability #10118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jbachorik wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve TempLocationManagerTest stability #10118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c245f2
Improve TempLocationManagerTest.java stability
jbachorik Dec 9, 2025
739ec92
Merge branch 'master' into jb/tlm_test_stability
jbachorik Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions internal-api/src/main/java/datadog/trace/util/TempLocationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,28 +40,27 @@ private static final class SingletonHolder {
private static final TempLocationManager INSTANCE = new TempLocationManager();
}

interface CleanupHook extends FileVisitor<Path> {
interface CleanupHook {
CleanupHook EMPTY = new CleanupHook() {};

@Override
default FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
default FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs, boolean timeout)
throws IOException {
return null;
return timeout ? FileVisitResult.TERMINATE : FileVisitResult.CONTINUE;
}

@Override
default FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
return null;
default FileVisitResult visitFile(Path file, BasicFileAttributes attrs, boolean timeout)
throws IOException {
return timeout ? FileVisitResult.TERMINATE : FileVisitResult.CONTINUE;
}

@Override
default FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
return null;
default FileVisitResult visitFileFailed(Path file, IOException exc, boolean timeout)
throws IOException {
return timeout ? FileVisitResult.TERMINATE : FileVisitResult.CONTINUE;
}

@Override
default FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
return null;
default FileVisitResult postVisitDirectory(Path dir, IOException exc, boolean timeout)
throws IOException {
return timeout ? FileVisitResult.TERMINATE : FileVisitResult.CONTINUE;
}

default void onCleanupStart(long timeout, TimeUnit unit) {}
Expand Down Expand Up @@ -99,10 +98,11 @@ public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
if (isTimedOut()) {
log.debug("Cleaning task timed out");
terminated = true;
cleanupTestHook.preVisitDirectory(dir, attrs, true);
return FileVisitResult.TERMINATE;
}

cleanupTestHook.preVisitDirectory(dir, attrs);
cleanupTestHook.preVisitDirectory(dir, attrs, false);

if (dir.equals(baseTempDir)) {
return FileVisitResult.CONTINUE;
Expand All @@ -128,9 +128,10 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
if (isTimedOut()) {
log.debug("Cleaning task timed out");
terminated = true;
cleanupTestHook.visitFile(file, attrs, true);
return FileVisitResult.TERMINATE;
}
cleanupTestHook.visitFile(file, attrs);
cleanupTestHook.visitFile(file, attrs, false);
try {
if (Files.getLastModifiedTime(file).toInstant().isAfter(cutoff)) {
return FileVisitResult.SKIP_SUBTREE;
Expand All @@ -147,9 +148,10 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOExce
if (isTimedOut()) {
log.debug("Cleaning task timed out");
terminated = true;
cleanupTestHook.visitFileFailed(file, exc, true);
return FileVisitResult.TERMINATE;
}
cleanupTestHook.visitFileFailed(file, exc);
cleanupTestHook.visitFileFailed(file, exc, false);
// do not log files/directories removed by another process running concurrently
if (!(exc instanceof NoSuchFileException) && log.isDebugEnabled()) {
log.debug("Failed to delete file {}", file, exc);
Expand All @@ -162,9 +164,10 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
if (isTimedOut()) {
log.debug("Cleaning task timed out");
terminated = true;
cleanupTestHook.postVisitDirectory(dir, exc, true);
return FileVisitResult.TERMINATE;
}
cleanupTestHook.postVisitDirectory(dir, exc);
cleanupTestHook.postVisitDirectory(dir, exc, false);
if (exc instanceof NoSuchFileException) {
return FileVisitResult.CONTINUE;
}
Expand Down
110 changes: 74 additions & 36 deletions internal-api/src/test/java/datadog/trace/util/TempLocationManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,29 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import datadog.trace.api.config.ProfilingConfig;
import datadog.trace.bootstrap.config.provider.ConfigProvider;
import datadog.trace.test.util.Flaky;
import java.io.IOException;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFilePermissions;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Properties;
import java.util.UUID;
import java.util.concurrent.Phaser;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.LockSupport;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -145,33 +148,40 @@ void testConcurrentCleanup(String section) throws Exception {

Phaser phaser = new Phaser(2);

Duration phaserTimeout =
Duration.ofSeconds(5); // wait at most 5 seconds for phaser coordination
AtomicBoolean withTimeout = new AtomicBoolean(false);
TempLocationManager.CleanupHook blocker =
new TempLocationManager.CleanupHook() {
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
throws IOException {
public FileVisitResult preVisitDirectory(
Path dir, BasicFileAttributes attrs, boolean timeout) throws IOException {
if (section.equals("preVisitDirectory") && dir.equals(fakeTempDir)) {
phaser.arriveAndAwaitAdvance();
phaser.arriveAndAwaitAdvance();
withTimeout.compareAndSet(false, timeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
}
return null;
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs, boolean timeout)
throws IOException {
if (section.equals("visitFile") && file.equals(fakeTempFile)) {
phaser.arriveAndAwaitAdvance();
phaser.arriveAndAwaitAdvance();
withTimeout.compareAndSet(false, timeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
}
return null;
}

@Override
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
public FileVisitResult postVisitDirectory(Path dir, IOException exc, boolean timeout)
throws IOException {
if (section.equals("postVisitDirectory") && dir.equals(fakeTempDir)) {
phaser.arriveAndAwaitAdvance();
phaser.arriveAndAwaitAdvance();
withTimeout.compareAndSet(false, timeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
arriveOrAwaitAdvance(phaser, phaserTimeout);
}
return null;
}
Expand All @@ -180,55 +190,63 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
TempLocationManager mgr = instance(baseDir, true, blocker);

// wait for the cleanup start
phaser.arriveAndAwaitAdvance();
Files.deleteIfExists(fakeTempFile);
phaser.arriveAndAwaitAdvance();
if (arriveOrAwaitAdvance(phaser, phaserTimeout)) {
Files.deleteIfExists(fakeTempFile);
assertTrue(arriveOrAwaitAdvance(phaser, phaserTimeout));
}
assertFalse(
withTimeout.get()); // if the cleaner times out it does not make sense to continue here
mgr.waitForCleanup(30, TimeUnit.SECONDS);

assertFalse(Files.exists(fakeTempFile));
assertFalse(Files.exists(fakeTempDir));
}

@Flaky("https://datadoghq.atlassian.net/browse/PROF-11290")
@ParameterizedTest
@MethodSource("timeoutTestArguments")
void testCleanupWithTimeout(boolean selfCleanup, boolean shouldSucceed, String section)
throws Exception {
void testCleanupWithTimeout(boolean shouldSucceed, String section) throws Exception {
long timeoutMs = 10;
AtomicBoolean withTimeout = new AtomicBoolean(false);
TempLocationManager.CleanupHook delayer =
new TempLocationManager.CleanupHook() {
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
throws IOException {
public FileVisitResult preVisitDirectory(
Path dir, BasicFileAttributes attrs, boolean timeout) throws IOException {
withTimeout.compareAndSet(false, timeout);
if (section.equals("preVisitDirectory")) {
LockSupport.parkNanos(timeoutMs * 1_000_000);
waitFor(Duration.ofMillis(timeoutMs));
}
return TempLocationManager.CleanupHook.super.preVisitDirectory(dir, attrs);
return TempLocationManager.CleanupHook.super.preVisitDirectory(dir, attrs, timeout);
}

@Override
public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException {
public FileVisitResult visitFileFailed(Path file, IOException exc, boolean timeout)
throws IOException {
withTimeout.compareAndSet(false, timeout);
if (section.equals("visitFileFailed")) {
LockSupport.parkNanos(timeoutMs * 1_000_000);
waitFor(Duration.ofMillis(timeoutMs));
}
return TempLocationManager.CleanupHook.super.visitFileFailed(file, exc);
return TempLocationManager.CleanupHook.super.visitFileFailed(file, exc, timeout);
}

@Override
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
public FileVisitResult postVisitDirectory(Path dir, IOException exc, boolean timeout)
throws IOException {
withTimeout.compareAndSet(false, timeout);
if (section.equals("postVisitDirectory")) {
LockSupport.parkNanos(timeoutMs * 1_000_000);
waitFor(Duration.ofMillis(timeoutMs));
}
return TempLocationManager.CleanupHook.super.postVisitDirectory(dir, exc);
return TempLocationManager.CleanupHook.super.postVisitDirectory(dir, exc, timeout);
}

@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs, boolean timeout)
throws IOException {
withTimeout.compareAndSet(false, timeout);
if (section.equals("visitFile")) {
LockSupport.parkNanos(timeoutMs * 1_000_000);
waitFor(Duration.ofMillis(timeoutMs));
}
return TempLocationManager.CleanupHook.super.visitFile(file, attrs);
return TempLocationManager.CleanupHook.super.visitFile(file, attrs, timeout);
}
};
Path baseDir =
Expand All @@ -244,16 +262,15 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
boolean rslt =
instance.cleanup((long) (timeoutMs * (shouldSucceed ? 20 : 0.5d)), TimeUnit.MILLISECONDS);
assertEquals(shouldSucceed, rslt);
assertNotEquals(shouldSucceed, withTimeout.get()); // timeout = !shouldSucceed
}

private static Stream<Arguments> timeoutTestArguments() {
List<Arguments> argumentsList = new ArrayList<>();
for (boolean selfCleanup : new boolean[] {true, false}) {
for (String intercepted :
new String[] {"preVisitDirectory", "visitFile", "postVisitDirectory"}) {
argumentsList.add(Arguments.of(selfCleanup, true, intercepted));
argumentsList.add(Arguments.of(selfCleanup, false, intercepted));
}
for (String intercepted :
new String[] {"preVisitDirectory", "visitFile", "postVisitDirectory"}) {
argumentsList.add(Arguments.of(true, intercepted));
argumentsList.add(Arguments.of(false, intercepted));
}
return argumentsList.stream();
}
Expand All @@ -270,4 +287,25 @@ private TempLocationManager instance(
return new TempLocationManager(
ConfigProvider.withPropertiesOverride(props), withStartupCleanup, cleanupHook);
}

private void waitFor(Duration timeout) {
long target = System.nanoTime() + timeout.toNanos();
while (System.nanoTime() < target) {
long toSleep = target - System.nanoTime();
if (toSleep > 0) {
LockSupport.parkNanos(toSleep);
}
}
}

private boolean arriveOrAwaitAdvance(Phaser phaser, Duration timeout) {
try {
System.err.println("===> waiting " + phaser.getPhase());
phaser.awaitAdvanceInterruptibly(phaser.arrive(), timeout.toMillis(), TimeUnit.MILLISECONDS);
System.err.println("===> done waiting " + phaser.getPhase());
return true;
} catch (InterruptedException | TimeoutException ignored) {
return false;
}
}
}