3 Commits

Author SHA1 Message Date
tkrmagid
41c7c48825 v0.4.21: extract publishIfNotCancelled, close main download publish race
Some checks failed
build / build (push) Has been cancelled
Reviewer-flagged: v0.4.20 fixed the reindex branch's post-put race, but
the main download completion path still did READY.put without a follow-up
epoch check. /videoCache clear landing between READY.put and the chat
notification could leave a resurrected READY entry pointing at a deleted
file, plus a stale "완료" message.

Extracted publishIfNotCancelled(url, path, startEpoch) helper that does
the full pre-check → put → post-check → rollback dance, and routed both
publish sites (reindex branch + download-complete branch) through it.
Centralizing this is the structural fix: future publish sites can't forget
the post-check.
2026-05-16 22:57:52 +09:00
tkrmagid
1913181d02 v0.4.20: close reindex publish race + harden preload guard
Some checks failed
build / build (push) Has been cancelled
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.
2026-05-16 22:54:20 +09:00
tkrmagid
d11289309b v0.4.19: fix post-move publish race in VideoCache.download
Some checks failed
build / build (push) Has been cancelled
Reviewer-flagged: v0.4.18 only checked epoch BEFORE Files.move. The window
between the move completing and READY.put / "완료" chat was still racy —
if /videoCache clear landed in that window, clearAll would epoch++ +
clear READY + delete files on disk, then the download thread would do
READY.put(url, finalPath) anyway, resurrecting a cleared entry and emitting
a stale "완료" message.

Add a second epoch check immediately AFTER Files.move(): if the epoch
changed, delete finalPath and return without publishing. The pre-move
check is kept too — it lets the common cancel-during-read case skip the
wasted move/delete round-trip.
2026-05-16 22:51:32 +09:00
2 changed files with 52 additions and 9 deletions

View File

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

View File

@@ -190,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;
@@ -229,6 +233,33 @@ public final class VideoCache {
// -- internals -----------------------------------------------------------------------
/**
* Publish {@code path} into {@link #READY} for {@code url}, but only if the cache
* epoch hasn't moved since {@code startEpoch} was captured. Wraps the
* pre-check / put / post-check / rollback dance so every publish site uses the
* same race-free pattern instead of re-implementing it.
*
* <p>Race coverage: if {@link #clearAll()} runs at any point between the
* pre-check and the post-check, the post-check sees the epoch bump and we
* roll back — {@link Map#remove(Object, Object)} is a compareAndRemove so we
* don't clobber a legitimate concurrent put under the same key, and we delete
* the on-disk file because clearAll's directory scan may have missed it (the
* file may not have existed yet when clearAll ran).
*
* @return {@code true} if the entry is now published; {@code false} if the
* caller's work was cancelled and {@code path} has been cleaned up.
*/
private static boolean publishIfNotCancelled(String url, Path path, long startEpoch) {
if (CACHE_EPOCH.get() != startEpoch) return false;
READY.put(url, path);
if (CACHE_EPOCH.get() != startEpoch) {
READY.remove(url, path);
try { Files.deleteIfExists(path); } catch (Throwable ignored) {}
return false;
}
return true;
}
private static void download(String url, long startEpoch) {
Path cacheDir = cacheDir();
Path partPath = null;
@@ -255,8 +286,11 @@ public final class VideoCache {
// 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);
if (!publishIfNotCancelled(url, finalPath, startEpoch)) {
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);
@@ -340,11 +374,10 @@ public final class VideoCache {
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.
// 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 at publish (clearAll ran) — {}",
VideoPlayerMod.LOG.info("[{}] preload: cancelled before move (clearAll ran) — {}",
VideoPlayerMod.MOD_ID, url);
try { Files.deleteIfExists(partPath); } catch (Throwable ignored) {}
return;
@@ -352,7 +385,17 @@ public final class VideoCache {
Files.move(partPath, finalPath, StandardCopyOption.REPLACE_EXISTING,
StandardCopyOption.ATOMIC_MOVE);
READY.put(url, finalPath);
// Race-safe publish: this covers the window between Files.move and READY.put
// (clearAll deletes the freshly-moved file then we resurrect it in READY) AND
// the window between READY.put and the "완료" notification (clearAll wipes
// READY then we re-publish + emit stale completion chat). publishIfNotCancelled
// rolls back both READY and the on-disk file if the epoch moved at any time.
if (!publishIfNotCancelled(url, finalPath, startEpoch)) {
VideoPlayerMod.LOG.info("[{}] preload: cancelled at publish (clearAll ran) — {}",
VideoPlayerMod.MOD_ID, url);
return;
}
VideoPlayerMod.LOG.info("[{}] preload: cached {} ({} bytes) -> {}",
VideoPlayerMod.MOD_ID, url, total, finalPath.getFileName());
long mb = Math.max(1, total / (1024 * 1024));