v0.4.20: close reindex publish race + harden preload guard

Reviewer-flagged: the existing-cache reindex branch in VideoCache.download
only ran one epoch check before READY.put, so a /videoCache clear landing
between the check and the put could leave a stale entry pointing at a
now-deleted file. Same pattern as the post-move fix in v0.4.19, applied
to the reindex path: pre-check, put, post-check + rollback on mismatch.

Also: preload() previously gated on READY.containsKey(url), which silently
blocks a re-preload if READY holds a stale key whose backing file is gone
(e.g. user deleted the file manually, or the cleanup half of a clear race).
Switched to lookup(url) — same intent, but lookup verifies the file
actually exists on disk, so stale keys self-heal on the next preload.

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.20
 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);
@@ -158,7 +190,11 @@ public final class VideoCache {
     public static void preload(String url) {
         if (url == null || url.isEmpty()) return;
         if (!(url.startsWith("http://") || url.startsWith("https://"))) return;
-        if (READY.containsKey(url)) {
+        // Use lookup() (which also verifies the file exists on disk) instead of
+        // READY.containsKey — defends against READY containing a stale entry whose
+        // backing file was removed by clearAll() / a user-side file delete. Without this,
+        // a stale key would silently block a re-preload.
+        if (lookup(url) != null) {
             VideoPlayerMod.LOG.info("[{}] preload: already cached {}", VideoPlayerMod.MOD_ID, url);
             notifyChat("[videopreload] 이미 캐시됨: " + url, ChatFormatting.GRAY);
             return;
@@ -169,7 +205,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,25 +233,48 @@ 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) {
+                // Pre-publish check — bail if clearAll has run since submit.
+                if (CACHE_EPOCH.get() != startEpoch) return;
                 READY.put(url, finalPath);
+                // Post-publish re-check: same window as the post-move check on the download
+                // path. If clearAll landed between the epoch read above and the READY.put,
+                // it would have wiped READY + the file, and our put() resurrected a stale
+                // entry pointing at a now-deleted path. Roll it back: remove our entry
+                // (compareAndRemove via remove(key, value) to avoid clobbering a concurrent
+                // legitimate re-put) and best-effort delete the file.
+                if (CACHE_EPOCH.get() != startEpoch) {
+                    READY.remove(url, finalPath);
+                    try { Files.deleteIfExists(finalPath); } catch (Throwable ignored) {}
+                    VideoPlayerMod.LOG.info("[{}] preload: reindex cancelled (clearAll ran) — {}",
+                        VideoPlayerMod.MOD_ID, url);
+                    return;
+                }
                 VideoPlayerMod.LOG.info("[{}] preload: indexed existing cache {} -> {}",
                     VideoPlayerMod.MOD_ID, url, finalPath.getFileName());
                 notifyChat("[videopreload] 기존 캐시 사용: " + url, ChatFormatting.GREEN);
@@ -250,21 +313,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,17 +344,46 @@ 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;
+            }
+
+            // Pre-move cancellation check. If clearAll ran during the read loop, abort
+            // before promoting .part to final — saves a wasted move + delete.
+            if (CACHE_EPOCH.get() != startEpoch) {
+                VideoPlayerMod.LOG.info("[{}] preload: cancelled before move (clearAll ran) — {}",
+                    VideoPlayerMod.MOD_ID, url);
+                try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
+                return;
+            }
 
             Files.move(partPath, finalPath, StandardCopyOption.REPLACE_EXISTING,
                 StandardCopyOption.ATOMIC_MOVE);
+
+            // Post-move cancellation check. clearAll() may have run between the pre-move
+            // check above and the move itself — in that case clearAll's directory scan
+            // either missed our file (it didn't exist yet) or saw the .part and skipped /
+            // failed to delete it. Either way, finalPath now exists on disk but the
+            // user-visible cache state is "cleared", so we must delete the file and skip
+            // both the READY.put and the "완료" chat. Without this check, a clear right at
+            // this window leaves a resurrected file in READY and a stale "완료" message.
+            if (CACHE_EPOCH.get() != startEpoch) {
+                VideoPlayerMod.LOG.info("[{}] preload: cancelled after move (clearAll ran) — {}",
+                    VideoPlayerMod.MOD_ID, url);
+                try { Files.deleteIfExists(finalPath); } catch (Throwable ignored) {}
+                return;
+            }
+
             READY.put(url, finalPath);
             VideoPlayerMod.LOG.info("[{}] preload: cached {} ({} bytes) -> {}",
                 VideoPlayerMod.MOD_ID, url, total, finalPath.getFileName());
@@ -293,6 +394,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);
         }