Don't lose the adb failure reason to the stdout/stderr close race - #59
Merged
Conversation
`has_ended` flips when adb's *stdout* closes, but the UI read `stderr_text()` and dropped the session in the same breath — while the separate stderr thread might not have finished draining. So a session that died with a reason (device offline, unknown command, "more than one device") often showed a bare "adb ended" and discarded the reason for good. Add Session::reap(): wait on the already-exited child and join the stderr worker so the captured stderr is complete before it's read. The UI reaps the session before reading the reason. Also fix flaky config tests (from the persistence PR): tempdir_new() handed out a shared directory that ini_migration_reads_main_ini's remove_dir_all() could wipe while parallel file-based tests were mid-run — give each call a unique dir.
This was referenced Aug 6, 2026
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.
Summary
has_endedflips when adb's stdout closes, but the UI readstderr_text()and dropped the session in the same breath — while the separate stderr thread might not have finished draining yet. So a session that died with a reason (device offline, unknown command, "more than one device") often showed a bare "adb ended" and discarded the reason permanently.Add
Session::reap(): wait on the already-exited child and join the stderr worker so the captured stderr is complete before it is read (unlikestop(), it doesn't kill — the process already exited). The UI reaps the session before reading the reason.Also (drive-by test fix)
The config tests added in the persistence PR were flaky:
tempdir_new()handed out a shared directory, andini_migration_reads_main_ini'sremove_dir_all()could wipe it while parallel file-based tests were mid-run. Each call now gets a unique dir. (Surfaced locally when the new adb test shifted thread timing.)Test plan
reap_drains_stderr_of_a_self_ended_session— after a self-ended session,reap()joins the stderr worker andstderr_text()returns the full reason