v0.4.18: fix /videoCache clear vs in-flight download race

clearAll() previously only wiped finalized files + the IN_FLIGHT set; any
download that was already running on the single-thread executor kept
writing past the clear, then atomically moved its .part to final and
re-published into READY — resurrecting one cache entry post-clear.

Fix: add an AtomicLong CACHE_EPOCH. preload() captures the epoch at submit
time; clearAll() bumps the epoch BEFORE wiping disk; download(url, epoch)
checks at three points (pre-start, in read loop, pre-publish) and bails if
the epoch moved, deleting its own .part only AFTER closing the output
stream (Windows can't delete an open file).

Phase ordering in clearAll() also matters: bump epoch first → drop
indexes → delete files. That way the in-flight download sees the
cancellation flag before the index wipe / file delete races can interact
with it.

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.configuration-cache=false
 
 # Mod
 mod_id=video_player
-mod_version=0.4.17
+mod_version=0.4.18
 maven_group=com.ejclaw.videoplayer
 archives_base_name=video_player
 

src/main/java/com/ejclaw/videoplayer/client/playback/VideoCache.java

@@ -22,6 +22,7 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * Client-side disk cache for preloaded HTTP video URLs. Driven by the {@code /videopreload}
@@ -42,6 +43,14 @@ public final class VideoCache {
     private static final Map<String, Path> READY = new ConcurrentHashMap<>();
     /** urls whose download is currently queued or in flight. */
     private static final Set<String> IN_FLIGHT = ConcurrentHashMap.newKeySet();
+    /**
+     * Monotonic cache-generation counter. {@link #clearAll()} bumps this; every download
+     * captures the value at submit time and bails (deletes its {@code .part} without
+     * publishing) if the epoch has moved since. This is the cancellation channel for the
+     * in-flight download — without it, {@code clearAll} only wipes already-finalized files
+     * while the running download keeps writing and re-publishes one entry post-clear.
+     */
+    private static final AtomicLong CACHE_EPOCH = new AtomicLong(0);
 
     /**
      * Single-thread executor that serializes all downloads. Cap enforcement (the total
@@ -121,11 +130,36 @@ public final class VideoCache {
 
     /**
      * Wipe the entire cache directory and drop both indexes. Sent in response to
-     * {@code /videoCache clear}. Best-effort per-file deletion; logs failures but doesn't
+     * {@code /videoCache clear}.
-     * abort on the first one. In-flight downloads aren't cancelled — they'll fail at the
+     *
-     * atomic-move step (target dir gone) and log a warning, which is fine.
+     * <p>Three phases that need to stay in this order:
+     * <ol>
+     *   <li>Bump the epoch first — that flips the cancellation flag for any in-flight
+     *       download. The download thread checks the epoch in the read loop and at the
+     *       move step, so after this point it will refuse to publish a new entry and
+     *       will delete its own {@code .part}.</li>
+     *   <li>Clear the in-memory indexes. (READY clears whatever was already finalized
+     *       pre-clear; IN_FLIGHT is wiped so a subsequent same-URL preload re-enters
+     *       the queue instead of getting deduped by stale state.)</li>
+     *   <li>Best-effort delete every regular file in the cache directory. We do this
+     *       last so that if the in-flight download was momentarily holding a {@code .part}
+     *       open, the listing here also catches its post-close residue (though normally
+     *       phase 1 → the download deletes its own part).</li>
+     * </ol>
+     *
+     * <p>Best-effort per-file deletion; logs failures but doesn't abort on the first one.
+     * On Windows the open {@code .part} may not be deletable here — phase 1's cancellation
+     * + the download's own cleanup-on-bail handles that case.
      */
     public static void clearAll() {
+        // Phase 1: signal cancellation to any in-flight download BEFORE wiping disk.
+        CACHE_EPOCH.incrementAndGet();
+
+        // Phase 2: drop indexes.
+        READY.clear();
+        IN_FLIGHT.clear();
+
+        // Phase 3: delete files on disk.
         int deleted = 0;
         int failed  = 0;
         try {
@@ -147,8 +181,6 @@ public final class VideoCache {
             VideoPlayerMod.LOG.warn("[{}] clearAll failed: {}",
                 VideoPlayerMod.MOD_ID, t.toString());
         }
-        READY.clear();
-        IN_FLIGHT.clear();
         VideoPlayerMod.LOG.info("[{}] clearAll: deleted={} failed={}",
             VideoPlayerMod.MOD_ID, deleted, failed);
         notifyChat("[videocache] 전체 캐시 삭제: " + deleted + "개 파일", ChatFormatting.YELLOW);
@@ -169,7 +201,11 @@ public final class VideoCache {
             return;
         }
         notifyChat("[videopreload] 다운로드 대기열 추가: " + url, ChatFormatting.YELLOW);
-        DOWNLOAD_POOL.submit(() -> download(url));
+        // Capture the current epoch at submit time. The download thread checks against
+        // CACHE_EPOCH later — any mismatch means clearAll() ran in between and this
+        // download must abort without publishing.
+        long epoch = CACHE_EPOCH.get();
+        DOWNLOAD_POOL.submit(() -> download(url, epoch));
     }
 
     /** Return the local cached file path for this URL, or {@code null} if not yet ready. */
@@ -193,24 +229,33 @@ public final class VideoCache {
 
     // -- internals -----------------------------------------------------------------------
 
-    private static void download(String url) {
+    private static void download(String url, long startEpoch) {
         Path cacheDir = cacheDir();
+        Path partPath = null;
         try {
             if (cacheDir == null) {
                 VideoPlayerMod.LOG.warn("[{}] preload: no game dir, skipping {}",
                     VideoPlayerMod.MOD_ID, url);
                 return;
             }
+            // Pre-flight cancellation check — if clearAll already ran between submit and now,
+            // skip the whole thing. Avoids creating directories / .part files post-clear.
+            if (CACHE_EPOCH.get() != startEpoch) {
+                VideoPlayerMod.LOG.info("[{}] preload: cancelled before start (clearAll ran) — {}",
+                    VideoPlayerMod.MOD_ID, url);
+                return;
+            }
             Files.createDirectories(cacheDir);
 
             String hash = sha256(url);
             String ext  = extensionFromUrl(url);
             Path finalPath = cacheDir.resolve(hash + ext);
-            Path partPath  = cacheDir.resolve(hash + ext + ".part");
+            partPath  = cacheDir.resolve(hash + ext + ".part");
 
             // Resume-friendly: if the file's already on disk from an earlier session, just
             // index it without re-downloading.
             if (Files.exists(finalPath) && Files.size(finalPath) > 0) {
+                if (CACHE_EPOCH.get() != startEpoch) return;
                 READY.put(url, finalPath);
                 VideoPlayerMod.LOG.info("[{}] preload: indexed existing cache {} -> {}",
                     VideoPlayerMod.MOD_ID, url, finalPath.getFileName());
@@ -250,21 +295,30 @@ public final class VideoCache {
             }
 
             long total = 0;
+            boolean cancelled = false;
             try (InputStream in = raw.getInputStream();
                  OutputStream out = Files.newOutputStream(partPath)) {
                 byte[] buf = new byte[64 * 1024];
                 int n;
                 while ((n = in.read(buf)) >= 0) {
+                    // Cancellation check inside the loop. Break (not return) so the try-with
+                    // closes the output stream first — on Windows, deleting an open .part can
+                    // fail with AccessDeniedException, so we always close before deleting.
+                    if (CACHE_EPOCH.get() != startEpoch) {
+                        cancelled = true;
+                        break;
+                    }
                     total += n;
                     if (total > MAX_BYTES) {
                         long capMb = MAX_BYTES / (1024 * 1024);
                         VideoPlayerMod.LOG.warn(
                             "[{}] preload: {} exceeded per-video {} MB cap; aborting",
                             VideoPlayerMod.MOD_ID, url, capMb);
-                        try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+                        // Same close-before-delete dance for Windows.
+                        cancelled = true;
                         notifyChat("[videopreload] 실패 (단일 영상 " + capMb + "MB 초과): " + url,
                             ChatFormatting.RED);
-                        return;
+                        break;
                     }
                     if (existingCacheBytes + total > MAX_CACHE_BYTES) {
                         long capMb = MAX_CACHE_BYTES / (1024 * 1024);
@@ -272,14 +326,29 @@ public final class VideoCache {
                         VideoPlayerMod.LOG.warn(
                             "[{}] preload: total-cache cap exceeded ({}>{} MB); aborting {}",
                             VideoPlayerMod.MOD_ID, usedMb, capMb, url);
-                        try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+                        cancelled = true;
                         notifyChat("[videopreload] 실패 (전체 캐시 " + capMb + "MB 초과): " + url,
                             ChatFormatting.RED);
-                        return;
+                        break;
                     }
                     out.write(buf, 0, n);
                 }
             }
+            // Now the .part file is closed — safe to delete on Windows.
+            if (cancelled) {
+                try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+                return;
+            }
+
+            // Final cancellation check before publishing. If clearAll ran during the read
+            // loop's last iteration (after the loop check, before this point), we still
+            // refuse to publish — otherwise we'd resurrect a cache entry post-clear.
+            if (CACHE_EPOCH.get() != startEpoch) {
+                VideoPlayerMod.LOG.info("[{}] preload: cancelled at publish (clearAll ran) — {}",
+                    VideoPlayerMod.MOD_ID, url);
+                try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+                return;
+            }
 
             Files.move(partPath, finalPath, StandardCopyOption.REPLACE_EXISTING,
                 StandardCopyOption.ATOMIC_MOVE);
@@ -293,6 +362,10 @@ public final class VideoCache {
                 VideoPlayerMod.MOD_ID, url, t.toString());
             notifyChat("[videopreload] 실패 (" + t.getClass().getSimpleName() + "): " + url,
                 ChatFormatting.RED);
+            // Best-effort cleanup of any leftover .part on the error path.
+            if (partPath != null) {
+                try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+            }
         } finally {
             IN_FLIGHT.remove(url);
         }