Fix race condition causing false ECONNREFUSED error - #77
Open
LandonMoran wants to merge 1 commit into
Open
Conversation
AdbDialogFragment.onDialogShow() launches mDNS discovery, a TCP port check, and a LiveData observer that can all call startAndDismiss() concurrently. When mDNS resolves a dynamic wireless debugging port and the user taps the neutral button (or tryTcpModePortFirst fires) around the same time, two StarterActivity instances launch with different ports. One succeeds (dynamic port) while the other fails with ECONNREFUSED (port 5555 not live), producing a spurious error dialog even though Shizuku started successfully. Fix: add a one-shot guard in startAndDismiss(). Validate the port first, then check the guard, then set it and stop mDNS discovery before launching StarterActivity. A plain Boolean is sufficient since all call sites are on the main thread (button click, LiveData observer, withContext(Dispatchers.Main)). Closes #70
|
🤖 AI Triage Confirmed in manager/src/main/java/moe/shizuku/manager/home/AdbDialogFragment.kt: adding the startCommitted guard and calling adbMdns.stop() prevents concurrent start attempts from launching multiple StarterActivity instances.
|
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.
Summary
Fixes a race condition in
AdbDialogFragmentwhere two concurrent paths can each launch a separateStarterActivity, producing a spurious ECONNREFUSED error dialog even though Shizuku successfully starts.Closes #70.
Root Cause
AdbDialogFragment.onDialogShow()starts three things simultaneously:adbMdns.start()) — resolves_adb-tls-connect._tcpand callsstartAndDismiss(discoveredPort)via LiveData observertryTcpModePortFirst()— if TCP mode is on and port 5555 is live, callsstartAndDismiss(5555)startAndDismiss(5555)startAndDismiss()has no idempotency guard. If mDNS resolves a dynamic wireless debugging port (e.g. 37021) around the same time the user taps the neutral button (ortryTcpModePortFirstfires), twoStarterActivityinstances are launched with different ports:The user sees the error, swipes the app away, reopens it, and finds Shizuku running — exactly matching the issue report.
Fix
Add a one-shot guard in
startAndDismiss():Port validation happens before the guard so an invalid port never locks out a valid second attempt.
adbMdns.stop()is called immediately after committing to stop further mDNS callbacks. A plainBooleanis sufficient — all three call sites (button click, LiveData observer,withContext(Dispatchers.Main)) execute on the main thread.adbMdns.stop()is already called inonDismiss(), andAdbMdns.stop()is idempotent (if (!running) return), so calling it earlier is safe.Changes
AdbDialogFragment.kt— 5 lines added (1 field + 4 lines instartAndDismiss)Testing
adbMdns.stop()was verified by reading the source.What still needs testing:
StarterActivitylaunches