Handle stale ADB stream frames during session reuse#207
Open
ducminh-phan wants to merge 1 commit into
Open
Conversation
Match OPEN responses by local stream id instead of assuming the next socket frame belongs to the new session. Stale OKAY/CLSE frames are ignored, and stale WRTE frames are acknowledged with their original stream ids then discarded. Update shell command handling so CLSE is no longer acknowledged with OKAY. Shell readers now only ACK WRTE frames, discard stale stream payloads, and close only on CLSE for the active stream. Add regression tests for stale OPEN frames, stale shell WRTE handling, and avoiding OKAY replies to CLSE.
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
1 participant
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.
What went wrong
adb_clientassumed the next frame read from a reused transport belonged to the command that was just sent.That is not always true. Over TCP/wireless debugging, old stream frames can arrive late after the client has already sent a new OPEN. When that happened, the next session could consume a stale OKAY, CLSE, or WRTE as if it were the response for the new stream, causing failures like:
Open session failed: responses used <x> for our local_id instead of <y>Open session failed: got CLSE in response instead of OKAYOpen session failed: got WRTE in response instead of OKAYThis was also reported in #175.
Shell handling also acknowledged every received frame before checking it, so it sent
OKAYforCLSE.CLSEis terminal and should not be acknowledged.Reference: https://android.googlesource.com/platform/packages/modules/adb/+/1cf2f017d312f73b3dc53bda85ef2610e35a80e9/docs/dev/protocol.md
The fix
open_session()now waits for theOKAYthat matches the new stream'slocal_idinstead of trusting the next frame on the socket.While opening a session:
OKAYandCLSEframes are ignoredWRTEframes are acknowledged with their original stream ids, then discardedCLSEor earlyWRTEfor the new stream still fails the openShell command handling now reads frames first and only sends
OKAYforWRTE.For shell sessions:
CLSEis no longer acknowledgedWRTEpayload is discarded instead of being written to the current command outputCLSEis ignoredCLSEfor the active streamI debugged and wrote the fix with the help of GPT-5.5. I also verified the behaviour manually.