ApiCachePersistenceTest flakes: a background cache write can land in the @TempDir while JUnit is deleting it

[ashleycaselli]

ApiCachePersistenceTest.entryStoreRoundTrip failed once in four full mvn test runs on an unrelated branch, with no assertion failure — the test body passed and the cleanup blew up:

org.junit.platform.commons.JUnitException: Failed to close extension context
Caused by: org.junit.jupiter.api.io.TempDirDeletionStrategy$DeletionException:
    Failed to delete temp directory /var/folders/…/junit-15212631685136957705.
    The following paths could not be deleted: <root>, store
  Suppressed: java.nio.file.DirectoryNotEmptyException: …/junit-15212631685136957705
  Suppressed: java.nio.file.DirectoryNotEmptyException: …/junit-15212631685136957705/store

DirectoryNotEmptyException on store means a file was inside it at delete time — a file the test itself did not leave there.

Where it comes from

entryStoreDir is static, JVM-global state:

• ApiCachePersistence.java:228 — initEntryStore(File dir) just assigns entryStoreDir = dir.
• ApiCachePersistence.java:244 — storeEntry opens with File dir = entryStoreDir; if (dir == null) return;, then writes <dir>/<name>.tmp.<threadId>.<nanoTime> and moves it into place.

And storeEntry is reached from background threads:

NanodashThreadPool.submit(…)          ApiCache.java:443
  → Thread.sleep(100 + rand(400)), plus waitOutIngestDelay
  → ApiCache.updateResponse(…)        ApiCache.java:292
  → ApiCachePersistence.storeEntry(…) ApiCache.java:304

Surefire runs the whole suite in one JVM, so a refresh submitted by any test — including one in an earlier class — sleeps at least ~100 ms and then writes into whatever entryStoreDir points at when it gets there. While ApiCachePersistenceTest is running, that is the test's own @TempDir.

@AfterEach does call initEntryStore(null), but that only closes half the window. A pool task that already executed File dir = entryStoreDir is holding its own reference and keeps writing regardless — and @TempDir deletion runs after @AfterEach, so that write lands exactly while JUnit is walking the tree.

That background traffic is real in this suite, not hypothetical; the same full run logged

ERROR ApiResultComponent - Timed out waiting for API response for …/get-newer-versions-of-np?np=…

from a different test class, i.e. refreshes in flight past the test that started them.

Frequency

Seen once in four full-suite runs. ApiCachePersistenceTest on its own passes 3/3, which fits: in isolation there is no other class submitting refreshes.

Possible fixes

1. Make disabling the store actually stop writers. Have storeEntry take the same lock initEntryStore does, or re-check entryStoreDir after serializing and before the move, so a captured directory reference cannot outlive initEntryStore(null). This is the only fix that helps production too — the same race exists against shutdown().
2. Quiesce the pool in teardown. Wait for NanodashThreadPool to drain in @AfterEach, before @TempDir cleanup runs.
3. Stop using @TempDir here. A directory the test creates and deletes leniently would make a late write harmless rather than fatal.

(1) plus (2) looks like the right pair: (1) removes the race, (2) keeps the test honest if some other writer appears later.

Notes

Not caused by, and not blocking, #666 — noticed while running that branch's full suite. Nothing in the flake touches the fixtures that PR moves.

🤖 Generated with Claude Code

https://claude.ai/code/session_014dByNErNfuj4au4r5vMsbf