Skip to content

fix(FarmTreeRun): build bank withdrawal list locally to avoid CME (1.2.3) - #639

Merged
chsami merged 1 commit into
developmentfrom
claude/err-farmtreerun-bank-items-cme
Oct 15, 2026
Merged

chsami merged 1 commit into
developmentfrom
claude/err-farmtreerun-bank-items-cme

Conversation

@chsami

@chsami chsami commented Oct 15, 2026

Copy link
Copy Markdown
Owner

Problem

Client error telemetry group 1915 (1 event, 1 reporter, Microbot 2.6.32, FarmTreeRunPlugin 1.2.2, first and last seen 2026-10-15) reports a java.util.ConcurrentModificationException on thread FarmTreeRunScript-1:

java.util.ArrayList$Itr.checkForComodification:1013
java.util.ArrayList$Itr.next:967
net.runelite.client.plugins.microbot.farmtreerun.FarmTreeRunScript.bank:748
net.runelite.client.plugins.microbot.farmtreerun.FarmTreeRunScript.lambda$run$2:150

Root cause

FarmTreeRunScript kept a single per-instance items list. bank() iterates it at line 748 (for (FarmingItem item : items)), and every iteration blocks on the client thread through getLinkedId(). While that loop runs, shutdown() (called from FarmTreeRunPlugin.shutDown() on the plugin-stop thread) runs items.clear() on the same list. A second scheduled loop on the same instance would also have shared the list. Either writer changing the list during the loop throws the CME. The line numbers match origin/development (6f54d34) exactly.

Change

  • Removed the items field. bank() now builds a local List<FarmingItem> items = new ArrayList<>(), so nothing is shared across threads.
  • addAvailableTravelItem and addFirstTravelItem take that list as their first parameter.
  • Moved the existing merge-by-itemId-and-noted logic, unchanged, into a package-private static List<FarmingItem> mergeItems(List<FarmingItem>) so it can be unit tested.
  • shutdown() no longer clears the list, because the list no longer exists outside bank().
  • Bumped FarmTreeRunPlugin from 1.2.2 to 1.2.3.

Validation

  • New FarmTreeRunBankItemsTest:
    • fails if FarmTreeRunScript declares a non-static Collection field again (the removed items field fails it);
    • checks that mergeItems sums entries with the same id and noted flag, keeps noted and unnoted apart, preserves order, and returns a new list without changing the input.
  • ./gradlew test --tests '*FarmTreeRunBankItemsTest' --tests '*PluginManifestVersionTest' -PpluginList=FarmTreeRunPlugin passes.

Known gaps

  • Not live-tested in game. Only one event so far.
  • Small behaviour change: if the plugin is stopped partway through banking, the old code usually aborted through the CME. Now bank() can still reach Rs2Bank.depositAllExcept before the isRunning() checks in the withdraw loops. The scheduled future is cancelled on stop, so the impact is low.
  • run() still overwrites mainScheduledFuture without cancelling an earlier one. With the list now local, a second loop can no longer cause this CME, so that change was left out.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV

…2.3)

bank() iterated the shared items field while the stop path cleared it from
the plugin-stop thread, throwing ConcurrentModificationException. The
withdrawal list is now a local variable passed to the travel-item helpers,
the merge is a static helper, and stopping the script no longer touches it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LjtEMzQHGuq4M1Mpqk9pPV
@chsami
chsami merged commit 351284e into development Oct 15, 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