fix(breakhandler): synchronize login lifecycle#1572
fix(breakhandler): synchronize login lifecycle#1572chsami wants to merge 1 commit intodevelopmentfrom
Conversation
WalkthroughThis pull request adds concurrency control mechanisms to prevent overlapping login attempts across the AutoLoginScript and BreakHandlerScript. AutoLoginScript now acquires a login lock at the start of Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/accountselector/AutoLoginScript.java(4 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T07:26:53.623Z
Learnt from: Voxsylvae
PR: chsami/Microbot#1415
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/pluginscheduler/SchedulerPlugin.java:2341-2366
Timestamp: 2025-08-25T07:26:53.623Z
Learning: In SchedulerPlugin.startLoginMonitoringThread(), do not gate startup on isBreakHandlerEnabled(); rely on isOnBreak() to suppress the monitor only when a break is actually active. This avoids blocking login monitoring whenever BreakHandler is merely enabled.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/accountselector/AutoLoginScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(52-1795)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(52-1795)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Build
| BreakHandlerState oldState = currentState.get(); | ||
| if (oldState != newState) { | ||
| log.debug("State transition: {} -> {}", oldState, newState); | ||
| currentState.set(newState); | ||
| stateChangeTime.set(Instant.now()); | ||
| retryCount.set(0); | ||
|
|
||
| // Reset safe condition wait time when leaving BREAK_REQUESTED | ||
| if (newState != BreakHandlerState.BREAK_REQUESTED) { | ||
| safeConditionWaitStartTime = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Preserve the login retry counter between LOGIN_REQUESTED ↔ LOGGING_IN hops
Resetting retryCount inside transitionToStateInternal now fires every time we bounce between LOGGING_IN and LOGIN_REQUESTED. As a result handleLoggingInState always reads currentRetryCount == 0, the "retry N of max" log never increments past 1, and we never hit getMaxLoginRetries(). That turns the login limiter into a no-op and lets us hammer the login flow indefinitely. Keep the counter when shuttling between those two states (reset everywhere else) so the retry guard can actually trip.
- retryCount.set(0);
+ if (!((oldState == BreakHandlerState.LOGGING_IN && newState == BreakHandlerState.LOGIN_REQUESTED)
+ || (oldState == BreakHandlerState.LOGIN_REQUESTED && newState == BreakHandlerState.LOGGING_IN))) {
+ retryCount.set(0);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BreakHandlerState oldState = currentState.get(); | |
| if (oldState != newState) { | |
| log.debug("State transition: {} -> {}", oldState, newState); | |
| currentState.set(newState); | |
| stateChangeTime.set(Instant.now()); | |
| retryCount.set(0); | |
| // Reset safe condition wait time when leaving BREAK_REQUESTED | |
| if (newState != BreakHandlerState.BREAK_REQUESTED) { | |
| safeConditionWaitStartTime = null; | |
| } | |
| } | |
| BreakHandlerState oldState = currentState.get(); | |
| if (oldState != newState) { | |
| log.debug("State transition: {} -> {}", oldState, newState); | |
| currentState.set(newState); | |
| stateChangeTime.set(Instant.now()); | |
| if (!((oldState == BreakHandlerState.LOGGING_IN && newState == BreakHandlerState.LOGIN_REQUESTED) | |
| || (oldState == BreakHandlerState.LOGIN_REQUESTED && newState == BreakHandlerState.LOGGING_IN))) { | |
| retryCount.set(0); | |
| } | |
| // Reset safe condition wait time when leaving BREAK_REQUESTED | |
| if (newState != BreakHandlerState.BREAK_REQUESTED) { | |
| safeConditionWaitStartTime = null; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java
around lines 755 to 766, the code unconditionally resets retryCount on every
state transition which wipes the login retry counter when bouncing between
LOGIN_REQUESTED and LOGGING_IN; change the reset logic so retryCount is NOT
reset when transitioning between LOGIN_REQUESTED and LOGGING_IN in either
direction, but is still reset for all other state transitions (keep existing
resets of stateChangeTime and safeConditionWaitStartTime behavior unchanged).
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f11da370588328be530d0bdc1a7782