Repository navigation
fix: run swing mutations on the edt to stop client freezes - #116
Merged
Merged
Conversation
Widget/script events and the plugin''s repeating scheduler task mutated swing components off the edt. Switching views through FastTabGroup validates the component tree (taking the awt tree lock and reading text field documents) while the edt was simultaneously setting text on those same fields (holding the document write lock while waiting for the tree lock), closing a lock-order cycle that froze both the game client thread and the edt: no exception, no log output, whole client frozen. - FastTabGroup entry points (select, showView, selectPreviouslySelectedTab, revertToSafeDisplay) bounce to the edt, covering the WidgetLoaded and ScriptPostFired handlers, the ge history view and the offer editors - the 1s repeating task ui updates are batched into one invokeLater per tick instead of mutating labels/panels from the scheduler thread - handleLogin account selector updates and MasterPanel.setupAccSelectorDropdown (startup) bounce to the edt too reproduced the freeze on the unpatched client and captured thread dumps showing the cycle, then verified the same actions no longer freeze the patched client
AntonyGarand
marked this pull request as ready for review
October 21, 2026 15:10
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The client intermittently froze completely (no crash, no log output) when doing certain GE actions. Reproduced on a developer-mode client and captured thread dumps of the frozen JVM (
jcmd <pid> Thread.print -l, saved during diagnosis).Root cause: an AWT tree lock ↔ document lock deadlock
Two threads were taking the same two locks in opposite order:
Game
Clientthread (RuneLite deliversWidgetLoaded/ScriptPostFiredon it) — holds the AWT tree lock, wants a document lock:EDT — holds that document's write lock, wants the tree lock:
Once the cycle closes, both threads freeze forever: the game loop stops ticking (whole client frozen) and nothing throws, which is why nothing lands in the logs. Note
jstack's own deadlock detector misses this cycle becausereadLock()waits insideObject.wait()(releasing the monitor), hiding the edge.CPU at freeze: ~10% of one core (not a spin — threads parked). The same audit found more off-EDT Swing mutations of the same class, listed below.
Fix
All Swing mutations now bounce to the EDT (no behavior change beyond threading):
FastTabGroup—select,showView,selectPreviouslySelectedTab,revertToSafeDisplayguard onSwingUtilities.isEventDispatchThread()and re-post viainvokeLater. This centrally fixes the game-thread paths:onWidgetLoaded(GE offers/bankpin →selectPreviouslySelectedTab, GE history →showView),onScriptPostFired(script 29), the offer editor tab selects fromonVarClientIntChanged, andhandleLogout'srevertToSafeDisplay.FlippingPlugin.setupRepeatingTasks) —updateTimerDisplays/updateTimeDisplay/updateSessionTime/updateAutoSaveDisplaywere mutating labels/panels from the scheduler thread (the frozen dumps showed it blocked inStatsPanel.updateAutoSaveDisplay → Container.add). They're now batched into oneinvokeLaterper tick. This also removes the cross-thread panel iteration that the oldConcurrentModificationException-restart hack was papering over.handleLogin— account selectorsetVisible/setSelectedItem(fires the listener that rebuilds all panels) bounced to the EDT; login runs on the client thread.MasterPanel.setupAccSelectorDropdown— called on the client thread during startup; guarded the same way.Already-EDT-safe paths verified and left untouched:
AbstractOfferEditorPanel.rebuild,GeHistoryTabPanel.rebuild,FlippingPanel.rebuild/highlightItem,StatsPanel.rebuildItemsDisplay, and the existingupdateAutoSaveDisplaycall sites inFlippingPlugin.Verification