Skip to content

fix(ShootingStar): close stop race on null panel and null state (1.4.11) - #652

Merged
chsami merged 2 commits into
developmentfrom
claude/err-shootingstar-stop-race-null-panel-and-state
Oct 17, 2026
Merged

chsami merged 2 commits into
developmentfrom
claude/err-shootingstar-stop-race-null-panel-and-state

Conversation

@chsami

@chsami chsami commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Problem

Client error telemetry for ShootingStarPlugin 1.4.10 (client 2.6.33) recorded 793 NullPointerExceptions across three groups. All three come from stopping the plugin:

  • 2529 (787 events): NullPointerException on the AWT EDT in ShootingStarPlugin.lambda$updatePanelList$3:419, where panel.updateList(stars) runs with panel already null.
  • 2532 (1 event): the same NPE in lambda$updatePanelList$4:423, from panel.refreshList(stars).
  • 2531 (5 events): NullPointerException in ShootingStarScript.lambda$run$1:128 on switch (state), after state had been set to null.

Root cause

  1. Panel NPE. When the player died and shutdownOnDeath was on, the death handler called shutDown() directly. That removed the panel and set panel to null, but the plugin was still registered with the event bus. Later events and game ticks kept calling updatePanelList, and each call queued an invokeLater lambda that dereferenced the null panel on the EDT. A runnable queued just before a normal stop could hit the same NPE.
  2. State NPE. Script.shutdown() calls mainScheduledFuture.cancel(true), and that does not wait for an iteration that is already running. ShootingStarScript.shutdown() then set state, currentStar and pickaxe to null, so the iteration still running read null fields at switch (state). The same could happen with currentStar in WALKING, MINING and BANKING.

Change

  • The death handler now calls Microbot.stopPlugin(this), so the plugin is stopped and unregistered the normal way. Called off the EDT, this does not block.
  • updatePanelList returns early when panel is null. The EDT lambda reads panel into a local once and skips the update if it is null.
  • shutdown() no longer clears state, currentStar or pickaxe. run() resets them at startup instead, next to the other per-run fields it already resets.
  • The overlay reads the static state into a local once before it null-checks and renders it.
  • Rs2Antiban.getCategory().isBusy() goes through a null-safe isAntibanBusy helper, because Rs2Antiban.resetAntibanSettings() sets the category to null.
  • Version bumped from 1.4.10 to 1.4.11.

Validation

  • Added ShootingStarStopRaceTest (5 tests). It covers a queued panel update after the panel is removed, which NPEs on the old lambdas. It also checks that shutdown() leaves the loop state alone for an iteration that is still running, which fails on code that clears the state on stop.
  • ./gradlew test --tests 'net.runelite.client.plugins.microbot.shootingstar.*' build -PpluginList=ShootingStarPlugin passes. That covers ShootingStarStopRaceTest (5), ShootingStarWalkBeforeHopTest (15) and ShootingStarWorldFilterTest (10).

Known gaps

  • The script executor is reused across restarts. If the plugin is stopped and then restarted straight away while an old iteration is still running, run() resetting state could still cause an NPE in that old iteration. The main loop catches and logs it, and it does no harm.
  • Rs2Antiban.resetAntibanSettings() setting the category to null is a client issue. JewelryScript and GlassblowingScript call Rs2Antiban.getCategory().isBusy() the same way and are not covered here. A client-side non-null default category would fix all of them.
  • Group 2530 (Rs2Bank.walkToBank NPE on a null bank location) arrived in the same upload. Client fix #1996 already covers it, so this PR does not touch it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV

chsami and others added 2 commits October 17, 2026 20:49
On "Oh dear, you are dead!" the plugin called its own shutDown()
directly, which removed and nulled the panel while the plugin stayed
registered, so tick, event and config handlers kept queueing
updatePanelList() runnables that dereferenced the null panel on the EDT.
Even a runnable queued just before a normal stop read the field after
removePanel() had cleared it.

The script loop null-checked the static state field and then switched
on it again after several client calls, so a concurrent stop that set
state to null threw on ordinal().

- stop through Microbot.stopPlugin(this) so the plugin is unregistered
- skip updatePanelList when there is no panel, and re-read the panel on
  the EDT so queued updates skip a removed panel
- read state into a local once per loop iteration and switch on it
- treat a missing antiban category as not busy

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV
ShootingStarScript.shutdown() nulled state, currentStar and pickaxe
while mainScheduledFuture.cancel(true) does not wait for a running
iteration, so a loop that had already passed its null check went on to
dereference the cleared fields (switch on state, currentStar in the
WALKING, MINING and BANKING cases). The previous local snapshot of state
only moved the NPE to the currentStar reads.

- stop clearing state, currentStar and pickaxe in shutdown()
- clear them at the top of run() with the other per-run resets
- drop the now unneeded state snapshot in the loop
- read the static state once in the overlay before rendering it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV
@chsami
chsami merged commit 10db07a into development Oct 17, 2026
1 check passed
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