Skip to content

Prevent closed menu holders from restarting refresh tasks - #307

Open
SevxLabs wants to merge 1 commit into
HelpChat:mainfrom
SevxLabs:fix/closed-menu-refresh-tasks
Open

SevxLabs wants to merge 1 commit into
HelpChat:mainfrom
SevxLabs:fix/closed-menu-refresh-tasks

Conversation

@SevxLabs

Copy link
Copy Markdown

Problem

Closing a menu cancels its repeating tasks, but does not invalidate refresh work that has already been queued. An asynchronous refresh can finish after closeMenu removes the holder and its main-thread completion can start a new placeholder update task. That task is no longer covered by the holder's normal close lifecycle.

This can result in repeated placeholder requests for disconnected viewers. PlaceholderAPI passes null to player-only expansions for offline players, which may produce repeating exceptions when an expansion assumes an online player.

Changes

  • Give each holder a terminal closed state and cancel both repeating tasks on normal close and shutdown.
  • Serialize task registration/cancellation with close so delayed work cannot register a new task after cancellation.
  • Guard refresh entry, queued selection, queued completion, and placeholder ticks against closed/offline holders.
  • Do not complete opening a menu if its viewer disconnected while it was being prepared.
  • Skip player placeholder parsing for offline viewer/target references, including placeholders inside menu arguments.
  • Handle an empty refresh in the guarded main-thread callback, rather than closing a potentially replaced menu from the asynchronous worker.

Regression test

The test scheduler queues a refresh, executes its asynchronous selection, closes the holder, and then executes the pending main-thread completion. The completion must not render items or restart a task. Removing the completion guard makes this test fail; restoring it passes.

Eight tests cover the two close-during-refresh phases, rejection of task starts for closed/offline holders, idempotent cancellation of both tasks, a queued tick after disconnect, offline placeholder targets, and normal online placeholder/item updates.

Validation

  • gradlew clean test shadowJar --no-daemon --console=plain passed on Java 21.
  • 8 tests passed, with no failures or skips.
  • Inspected the built jar for expected plugin metadata/classes and absence of test dependencies.
  • Existing Java target, configuration defaults, and runtime dependencies are unchanged; new dependencies are test-only.

This is a focused lifecycle fix, not a rewrite of all asynchronous menu operations, nor a general null-safety fix for third-party expansions. Validation uses mocked Bukkit objects and a controlled scheduler; real-server acceptance testing remains outstanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant