Add reentrancy guard, failure abort, and admin status endpoint to TempFileCleanupService

app/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java

@@ -3,10 +3,15 @@ package stirling.software.common.service;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
@@ -47,6 +52,29 @@ public class TempFileCleanupService {
     // Maximum recursion depth for directory traversal
     private static final int MAX_RECURSION_DEPTH = 5;
 
+    // After this many consecutive failures the scheduler skips runs until a successful
+    // cycle resets the counter. Stops the service from hammering a broken FS / NFS mount
+    // and spamming the log on every fixed-delay tick.
+    private static final int MAX_CONSECUTIVE_FAILURES = 10;
+
+    // Reentrancy guard: if a cleanup cycle outlasts the configured interval the next
+    // @Scheduled tick must skip rather than overlap. Spring's @Scheduled is single-threaded
+    // by default, but operators frequently override that pool to multi-threaded for other
+    // workloads and we do not want two cleanups racing on the same paths.
+    private final AtomicBoolean cleanupRunning = new AtomicBoolean(false);
+
+    // Observability state. Kept as primitives + atomic references so reads from
+    // getCleanupStatus() never race with a running cycle.
+    private final AtomicLong totalRuns = new AtomicLong(0);
+    private final AtomicLong totalSkipped = new AtomicLong(0);
+    private final AtomicLong totalFailures = new AtomicLong(0);
+    private final AtomicInteger consecutiveFailures = new AtomicInteger(0);
+    private final AtomicLong lastRunDurationMs = new AtomicLong(0);
+    private final AtomicLong lastDeletedCount = new AtomicLong(0);
+    private final AtomicReference<Instant> lastRunStartedAt = new AtomicReference<>(null);
+    private final AtomicReference<Instant> lastRunCompletedAt = new AtomicReference<>(null);
+    private final AtomicReference<String> lastErrorMessage = new AtomicReference<>(null);
+
     // File patterns that identify our temp files
     private static final Predicate<String> IS_OUR_TEMP_FILE =
             fileName ->
@@ -133,45 +161,143 @@ public class TempFileCleanupService {
                     "#{applicationProperties.system.tempFileManagement.cleanupIntervalMinutes}",
             timeUnit = TimeUnit.MINUTES)
     public void scheduledCleanup() {
-        log.info("Running scheduled temporary file cleanup");
-        long maxAgeMillis = tempFileManager.getMaxAgeMillis();
-
-        // Clean up registered temp files (managed by TempFileRegistry)
-        int registeredDeletedCount = tempFileManager.cleanupOldTempFiles(maxAgeMillis);
-        log.info("Cleaned up {} registered temporary files", registeredDeletedCount);
-
-        // Clean up registered temp directories
-        int directoriesDeletedCount = 0;
-        for (Path directory : registry.getTempDirectories()) {
-            try {
-                if (Files.exists(directory)) {
-                    GeneralUtils.deleteDirectory(directory);
-                    directoriesDeletedCount++;
-                    log.debug("Cleaned up temporary directory: {}", directory);
-                }
-            } catch (IOException e) {
-                log.warn("Failed to clean up temporary directory: {}", directory, e);
-            }
+        if (consecutiveFailures.get() >= MAX_CONSECUTIVE_FAILURES) {
+            // Once this latches operators must restart (or call resetCleanupFailureCounter via
+            // the management endpoint) to retry. The alternative - retrying forever - turned an
+            // unreachable NFS mount into a log-flood that masked unrelated issues.
+            totalSkipped.incrementAndGet();
+            log.warn(
+                    "Skipping scheduled cleanup: {} consecutive failures already recorded. "
+                            + "Investigate the temp directory before re-enabling.",
+                    consecutiveFailures.get());
+            return;
         }
 
-        // Clean up PDFBox cache file
-        cleanupPDFBoxCache();
+        if (!cleanupRunning.compareAndSet(false, true)) {
+            // Previous cycle still running. Skipping is correct: piling up overlapping cleanups
+            // amplifies the underlying problem (slow disk, huge backlog) rather than draining it.
+            totalSkipped.incrementAndGet();
+            log.warn(
+                    "Skipping scheduled cleanup: a previous cycle is still running"
+                            + " (last started at {})",
+                    lastRunStartedAt.get());
+            return;
+        }
 
-        // Clean up unregistered temp files based on our cleanup strategy
-        boolean containerMode = isContainerMode();
-        int unregisteredDeletedCount = cleanupUnregisteredFiles(containerMode, true, maxAgeMillis);
+        Instant start = Instant.now();
+        lastRunStartedAt.set(start);
+        try {
+            log.info("Running scheduled temporary file cleanup");
+            long maxAgeMillis = tempFileManager.getMaxAgeMillis();
 
-        if (registeredDeletedCount > 0
-                || unregisteredDeletedCount > 0
-                || directoriesDeletedCount > 0) {
-            log.info(
-                    "Scheduled cleanup complete. Deleted {} registered files, {} unregistered files, {} directories",
-                    registeredDeletedCount,
-                    unregisteredDeletedCount,
-                    directoriesDeletedCount);
+            // Clean up registered temp files (managed by TempFileRegistry)
+            int registeredDeletedCount = tempFileManager.cleanupOldTempFiles(maxAgeMillis);
+            log.info("Cleaned up {} registered temporary files", registeredDeletedCount);
+
+            // Clean up registered temp directories
+            int directoriesDeletedCount = 0;
+            for (Path directory : registry.getTempDirectories()) {
+                try {
+                    if (Files.exists(directory)) {
+                        GeneralUtils.deleteDirectory(directory);
+                        directoriesDeletedCount++;
+                        log.debug("Cleaned up temporary directory: {}", directory);
+                    }
+                } catch (IOException e) {
+                    log.warn("Failed to clean up temporary directory: {}", directory, e);
+                }
+            }
+
+            // Clean up PDFBox cache file
+            cleanupPDFBoxCache();
+
+            // Clean up unregistered temp files based on our cleanup strategy
+            boolean containerMode = isContainerMode();
+            int unregisteredDeletedCount =
+                    cleanupUnregisteredFiles(containerMode, true, maxAgeMillis);
+
+            int totalDeleted =
+                    registeredDeletedCount + unregisteredDeletedCount + directoriesDeletedCount;
+            lastDeletedCount.set(totalDeleted);
+
+            if (totalDeleted > 0) {
+                log.info(
+                        "Scheduled cleanup complete. Deleted {} registered files, {} unregistered"
+                                + " files, {} directories",
+                        registeredDeletedCount,
+                        unregisteredDeletedCount,
+                        directoriesDeletedCount);
+            }
+
+            consecutiveFailures.set(0);
+            lastErrorMessage.set(null);
+            totalRuns.incrementAndGet();
+        } catch (RuntimeException e) {
+            // Top-level catch so a transient failure (FS unmount, permission flip) does not
+            // poison the @Scheduled invocation chain. The inner loops already swallow per-file
+            // IOExceptions; this is the safety net for everything else.
+            int streak = consecutiveFailures.incrementAndGet();
+            totalFailures.incrementAndGet();
+            lastErrorMessage.set(e.getClass().getSimpleName() + ": " + e.getMessage());
+            log.error(
+                    "Scheduled cleanup failed (consecutive failure {}/{})",
+                    streak,
+                    MAX_CONSECUTIVE_FAILURES,
+                    e);
+        } finally {
+            Instant end = Instant.now();
+            lastRunCompletedAt.set(end);
+            lastRunDurationMs.set(Duration.between(start, end).toMillis());
+            cleanupRunning.set(false);
         }
     }
 
+    /**
+     * Snapshot of the cleanup loop's observability state. Safe to call from any thread; reads the
+     * atomic fields directly so no locking is required.
+     */
+    public CleanupStatus getCleanupStatus() {
+        return new CleanupStatus(
+                cleanupRunning.get(),
+                lastRunStartedAt.get(),
+                lastRunCompletedAt.get(),
+                lastRunDurationMs.get(),
+                lastDeletedCount.get(),
+                totalRuns.get(),
+                totalSkipped.get(),
+                totalFailures.get(),
+                consecutiveFailures.get(),
+                consecutiveFailures.get() >= MAX_CONSECUTIVE_FAILURES,
+                lastErrorMessage.get());
+    }
+
+    /**
+     * Reset the consecutive-failure counter so the scheduler resumes after a latched abort. Returns
+     * the prior streak so the caller can log what was cleared.
+     */
+    public int resetCleanupFailureCounter() {
+        int prior = consecutiveFailures.getAndSet(0);
+        lastErrorMessage.set(null);
+        if (prior > 0) {
+            log.info("Cleared {} consecutive cleanup failures; scheduler will resume", prior);
+        }
+        return prior;
+    }
+
+    /** Immutable status payload exposed via {@link #getCleanupStatus()} and the REST endpoint. */
+    public record CleanupStatus(
+            boolean running,
+            Instant lastRunStartedAt,
+            Instant lastRunCompletedAt,
+            long lastRunDurationMs,
+            long lastDeletedCount,
+            long totalRuns,
+            long totalSkipped,
+            long totalFailures,
+            int consecutiveFailures,
+            boolean abortLatched,
+            String lastError) {}
+
     /**
      * Perform startup cleanup of stale temporary files from previous runs. This is especially
      * important in Docker environments where temp files persist between container restarts.

app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java

@@ -522,6 +522,148 @@ public class TempFileCleanupServiceTest {
         }
     }
 
+    // ---------------------------------------------------------------------
+    // Cleanup resilience: reentrancy guard, failure counter, status snapshot
+    // ---------------------------------------------------------------------
+
+    @Test
+    public void getCleanupStatus_initialStateHasNoRecordedRun() {
+        TempFileCleanupService.CleanupStatus status = cleanupService.getCleanupStatus();
+
+        assertFalse(status.running());
+        assertNull(status.lastRunStartedAt());
+        assertNull(status.lastRunCompletedAt());
+        assertEquals(0L, status.lastRunDurationMs());
+        assertEquals(0L, status.lastDeletedCount());
+        assertEquals(0L, status.totalRuns());
+        assertEquals(0L, status.totalSkipped());
+        assertEquals(0L, status.totalFailures());
+        assertEquals(0, status.consecutiveFailures());
+        assertFalse(status.abortLatched());
+        assertNull(status.lastError());
+    }
+
+    @Test
+    public void scheduledCleanup_successfulRunUpdatesStatusAndClearsFailures() {
+        // Force a known failure first so we can prove it resets.
+        ReflectionTestUtils.setField(
+                cleanupService,
+                "consecutiveFailures",
+                new java.util.concurrent.atomic.AtomicInteger(3));
+        ReflectionTestUtils.setField(
+                cleanupService,
+                "lastErrorMessage",
+                new java.util.concurrent.atomic.AtomicReference<>("prior failure"));
+        when(tempFileManager.cleanupOldTempFiles(anyLong())).thenReturn(5);
+        when(registry.getTempDirectories()).thenReturn(new HashSet<>());
+
+        cleanupService.scheduledCleanup();
+
+        TempFileCleanupService.CleanupStatus status = cleanupService.getCleanupStatus();
+        assertFalse(status.running(), "cleanupRunning flag must reset in finally");
+        assertNotNull(status.lastRunStartedAt());
+        assertNotNull(status.lastRunCompletedAt());
+        assertEquals(1L, status.totalRuns());
+        assertEquals(0L, status.totalFailures());
+        assertEquals(0, status.consecutiveFailures(), "successful run must clear failure streak");
+        assertNull(status.lastError(), "successful run must clear the prior error");
+        assertFalse(status.abortLatched());
+    }
+
+    @Test
+    public void scheduledCleanup_runtimeExceptionIncrementsConsecutiveFailures() {
+        when(tempFileManager.cleanupOldTempFiles(anyLong()))
+                .thenThrow(new RuntimeException("disk unreachable"));
+
+        cleanupService.scheduledCleanup();
+
+        TempFileCleanupService.CleanupStatus status = cleanupService.getCleanupStatus();
+        assertEquals(1, status.consecutiveFailures());
+        assertEquals(1L, status.totalFailures());
+        assertEquals(0L, status.totalRuns(), "failing runs must not increment totalRuns");
+        assertNotNull(status.lastError());
+        assertTrue(status.lastError().contains("disk unreachable"));
+        assertFalse(status.running(), "cleanupRunning flag must reset even on failure");
+    }
+
+    @Test
+    public void scheduledCleanup_skipsWhenConsecutiveFailureLimitReached() {
+        ReflectionTestUtils.setField(
+                cleanupService,
+                "consecutiveFailures",
+                new java.util.concurrent.atomic.AtomicInteger(10));
+
+        cleanupService.scheduledCleanup();
+
+        TempFileCleanupService.CleanupStatus status = cleanupService.getCleanupStatus();
+        assertEquals(1L, status.totalSkipped(), "abort-latched run must count as skipped");
+        assertEquals(0L, status.totalRuns());
+        assertTrue(status.abortLatched());
+        // tempFileManager must not be called when the abort is latched
+        verify(tempFileManager, never()).cleanupOldTempFiles(anyLong());
+    }
+
+    @Test
+    public void resetCleanupFailureCounter_clearsCounterAndReturnsPriorValue() {
+        ReflectionTestUtils.setField(
+                cleanupService,
+                "consecutiveFailures",
+                new java.util.concurrent.atomic.AtomicInteger(7));
+        ReflectionTestUtils.setField(
+                cleanupService,
+                "lastErrorMessage",
+                new java.util.concurrent.atomic.AtomicReference<>("broken mount"));
+
+        int cleared = cleanupService.resetCleanupFailureCounter();
+
+        assertEquals(7, cleared);
+        TempFileCleanupService.CleanupStatus status = cleanupService.getCleanupStatus();
+        assertEquals(0, status.consecutiveFailures());
+        assertNull(status.lastError());
+        assertFalse(status.abortLatched());
+    }
+
+    @Test
+    public void scheduledCleanup_reentrancyGuardSkipsOverlappingCycle() throws Exception {
+        java.util.concurrent.CountDownLatch firstCycleStarted =
+                new java.util.concurrent.CountDownLatch(1);
+        java.util.concurrent.CountDownLatch releaseFirstCycle =
+                new java.util.concurrent.CountDownLatch(1);
+
+        when(tempFileManager.cleanupOldTempFiles(anyLong()))
+                .thenAnswer(
+                        invocation -> {
+                            firstCycleStarted.countDown();
+                            // Hold the cycle open so the second call sees cleanupRunning == true.
+                            assertTrue(
+                                    releaseFirstCycle.await(
+                                            5, java.util.concurrent.TimeUnit.SECONDS),
+                                    "test latch timed out");
+                            return 1;
+                        });
+        when(registry.getTempDirectories()).thenReturn(new HashSet<>());
+
+        Thread firstCycle = new Thread(cleanupService::scheduledCleanup, "test-cleanup-1");
+        firstCycle.start();
+        assertTrue(
+                firstCycleStarted.await(5, java.util.concurrent.TimeUnit.SECONDS),
+                "first cycle never reached the mocked cleanup call");
+
+        // Second invocation must short-circuit because cleanupRunning is still true.
+        cleanupService.scheduledCleanup();
+        TempFileCleanupService.CleanupStatus midRun = cleanupService.getCleanupStatus();
+        assertTrue(midRun.running(), "first cycle should still be running");
+        assertEquals(1L, midRun.totalSkipped(), "second call must skip without running");
+
+        releaseFirstCycle.countDown();
+        firstCycle.join(5_000);
+
+        TempFileCleanupService.CleanupStatus afterRun = cleanupService.getCleanupStatus();
+        assertFalse(afterRun.running());
+        assertEquals(1L, afterRun.totalRuns(), "only the first cycle should count as a real run");
+        assertEquals(1L, afterRun.totalSkipped(), "skip count must not change after release");
+    }
+
     // Matcher for exact path equality
     private static Path eq(Path path) {
         return argThat(arg -> arg != null && arg.equals(path));

app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/TempCleanupAdminController.java

@@ -0,0 +1,52 @@
+package stirling.software.proprietary.security.controller.api;
+
+import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.prepost.PreAuthorize;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+
+import io.swagger.v3.oas.annotations.Operation;
+
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+
+import stirling.software.common.annotations.api.AdminApi;
+import stirling.software.common.service.TempFileCleanupService;
+import stirling.software.common.service.TempFileCleanupService.CleanupStatus;
+
+/**
+ * Admin-only observability endpoints for the scheduled temp-file cleanup loop. Lets operators see
+ * the last cycle's outcome and clear the latched abort counter without restarting the JVM.
+ */
+@AdminApi
+@RequiredArgsConstructor
+@PreAuthorize("hasRole('ADMIN')")
+@Slf4j
+public class TempCleanupAdminController {
+
+    private final TempFileCleanupService tempFileCleanupService;
+
+    @GetMapping("/temp-cleanup/status")
+    @Operation(
+            summary = "Get temp file cleanup status",
+            description =
+                    "Returns the most recent cleanup cycle's start/end time, duration, deleted"
+                            + " count, and the consecutive-failure counter. Use this to confirm"
+                            + " the scheduler is making progress.")
+    public ResponseEntity<CleanupStatus> getStatus() {
+        return ResponseEntity.ok(tempFileCleanupService.getCleanupStatus());
+    }
+
+    @PostMapping("/temp-cleanup/reset-failures")
+    @Operation(
+            summary = "Reset the temp cleanup failure counter",
+            description =
+                    "Clears the consecutive-failure counter that latches the scheduler after"
+                            + " repeated cycle failures. Use after investigating and fixing the"
+                            + " underlying issue (e.g. unreachable mount, permissions).")
+    public ResponseEntity<CleanupStatus> resetFailures() {
+        int cleared = tempFileCleanupService.resetCleanupFailureCounter();
+        log.info("Admin manually cleared {} consecutive cleanup failures", cleared);
+        return ResponseEntity.ok(tempFileCleanupService.getCleanupStatus());
+    }
+}

app/proprietary/src/test/java/stirling/software/proprietary/security/controller/api/TempCleanupAdminControllerTest.java

@@ -0,0 +1,58 @@
+package stirling.software.proprietary.security.controller.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import java.time.Instant;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.ResponseEntity;
+
+import stirling.software.common.service.TempFileCleanupService;
+import stirling.software.common.service.TempFileCleanupService.CleanupStatus;
+
+@ExtendWith(MockitoExtension.class)
+class TempCleanupAdminControllerTest {
+
+    @Mock private TempFileCleanupService tempFileCleanupService;
+
+    @InjectMocks private TempCleanupAdminController controller;
+
+    @Test
+    void getStatus_returnsTheServiceSnapshot() {
+        Instant started = Instant.parse("2026-05-26T20:00:00Z");
+        Instant ended = Instant.parse("2026-05-26T20:00:01Z");
+        CleanupStatus expected =
+                new CleanupStatus(false, started, ended, 1234L, 42L, 7L, 1L, 0L, 0, false, null);
+        when(tempFileCleanupService.getCleanupStatus()).thenReturn(expected);
+
+        ResponseEntity<CleanupStatus> response = controller.getStatus();
+
+        assertThat(response.getStatusCode().value()).isEqualTo(200);
+        assertThat(response.getBody()).isSameAs(expected);
+        verify(tempFileCleanupService).getCleanupStatus();
+        verifyNoMoreInteractions(tempFileCleanupService);
+    }
+
+    @Test
+    void resetFailures_clearsTheCounterAndReturnsTheFreshSnapshot() {
+        CleanupStatus afterReset =
+                new CleanupStatus(false, null, null, 0L, 0L, 0L, 0L, 0L, 0, false, null);
+        when(tempFileCleanupService.resetCleanupFailureCounter()).thenReturn(5);
+        when(tempFileCleanupService.getCleanupStatus()).thenReturn(afterReset);
+
+        ResponseEntity<CleanupStatus> response = controller.resetFailures();
+
+        assertThat(response.getStatusCode().value()).isEqualTo(200);
+        assertThat(response.getBody()).isSameAs(afterReset);
+        verify(tempFileCleanupService).resetCleanupFailureCounter();
+        verify(tempFileCleanupService).getCleanupStatus();
+        verifyNoMoreInteractions(tempFileCleanupService);
+    }
+}