Context
From the July 2026 McRPG quest-system audit (adversarially verified: CONFIRMED, severity CRITICAL). Every player quit in McRPG currently registers the unload work as TWO 1-tick repeating Bukkit timers — one async, one on the main thread — producing concurrent duplicate chain flushes, synchronous JDBC on the main thread, and a leaked forever-ticking timer that pins the player object graph per quit.
Problem
PlayerUnloadTask's constructor self-schedules with runTask(true) (src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java:31). McRPG's quit listener then calls .runTask() (sync) on the same object.
RepeatableCoreTask.runTask(boolean) (task/core/RepeatableCoreTask.java:33) has no re-schedule guard (unlike CoreTask.runTask, which is overridden away), so the same Runnable registers as two timers and the second registration overwrites bukkitTaskId — the first timer's id is lost and can never be cancelled.
- Both timers share non-volatile state fields (
paused, cancelled, delayExpired, intervalStartTime).
CancelableCoreTask.cancelTask() (task/core/CancelableCoreTask.java:22) cancels only the tracked bukkitTaskId, so a lost registration outlives cancellation permanently.
Failure scenario: player quits → constructor registers async timer A; listener registers sync timer B (bukkitTaskId=B). Both pass the interval check in the same tick: main thread and async thread run the unload concurrently, racing on chain-state writes and dirty flags. Whichever finishes calls cancelTask() — only B is cancelled; A ticks forever holding the player graph.
Implementation plan
- Remove self-scheduling from the
PlayerUnloadTask constructor (callers schedule explicitly). Check PlayerLoadTask for the same pattern.
- Add an already-scheduled guard to
RepeatableCoreTask.runTask(boolean) mirroring CoreTask.runTask()'s taskExecuted guard — a second call must be a no-op (log a WARNING).
- Make task state fields (
paused, cancelled, delayExpired, and any read across threads) volatile.
- In
CancelableCoreTask, ensure cancellation covers every registration (track all task ids, or make the guard from step 2 structurally prevent multiple registrations).
- Audit other
task/player/ and task/core/ classes for constructor-side scheduling.
- Coordinate with the McRPG companion ticket that fixes the listener call sites — note the API behavior change in the PR description (downstream plugins relying on constructor self-scheduling would break; this repo's policy is deliberate, documented breaking changes only).
Acceptance criteria
Notes for the implementing session
Follow CLAUDE.md test conventions (Given/When/Then @DisplayName). One logical change per PR.
Context
From the July 2026 McRPG quest-system audit (adversarially verified: CONFIRMED, severity CRITICAL). Every player quit in McRPG currently registers the unload work as TWO 1-tick repeating Bukkit timers — one async, one on the main thread — producing concurrent duplicate chain flushes, synchronous JDBC on the main thread, and a leaked forever-ticking timer that pins the player object graph per quit.
Problem
PlayerUnloadTask's constructor self-schedules withrunTask(true)(src/main/java/com/diamonddagger590/mccore/task/player/PlayerUnloadTask.java:31). McRPG's quit listener then calls.runTask()(sync) on the same object.RepeatableCoreTask.runTask(boolean)(task/core/RepeatableCoreTask.java:33) has no re-schedule guard (unlikeCoreTask.runTask, which is overridden away), so the same Runnable registers as two timers and the second registration overwritesbukkitTaskId— the first timer's id is lost and can never be cancelled.paused,cancelled,delayExpired,intervalStartTime).CancelableCoreTask.cancelTask()(task/core/CancelableCoreTask.java:22) cancels only the trackedbukkitTaskId, so a lost registration outlives cancellation permanently.Failure scenario: player quits → constructor registers async timer A; listener registers sync timer B (bukkitTaskId=B). Both pass the interval check in the same tick: main thread and async thread run the unload concurrently, racing on chain-state writes and dirty flags. Whichever finishes calls
cancelTask()— only B is cancelled; A ticks forever holding the player graph.Implementation plan
PlayerUnloadTaskconstructor (callers schedule explicitly). CheckPlayerLoadTaskfor the same pattern.RepeatableCoreTask.runTask(boolean)mirroringCoreTask.runTask()'staskExecutedguard — a second call must be a no-op (log a WARNING).paused,cancelled,delayExpired, and any read across threads)volatile.CancelableCoreTask, ensure cancellation covers every registration (track all task ids, or make the guard from step 2 structurally prevent multiple registrations).task/player/andtask/core/classes for constructor-side scheduling.Acceptance criteria
runTasktwice on anyRepeatableCoreTaskschedules exactly one Bukkit task.CancelableCoreTaskleaves zero live registrations.ManagedExecutorExtensionvia@RegisterExtensionfor any executor needs; plain JUnit where Bukkit isn't required)../gradlew testpasses with zero failures.Notes for the implementing session
Follow
CLAUDE.mdtest conventions (Given/When/Then@DisplayName). One logical change per PR.