v0.4.18: fix /videoCache clear vs in-flight download race
Some checks failed
build / build (push) Has been cancelled
Some checks failed
build / build (push) Has been cancelled
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.
This commit is contained in:
@@ -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
|
||||
* 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.
|
||||
* {@code /videoCache clear}.
|
||||
*
|
||||
* <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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user