Skip to content

fix: background sessions follow-ups - #17

Open
oonid wants to merge 11 commits into
mainfrom
fix/background-sessions-followups
Open

oonid wants to merge 11 commits into
mainfrom
fix/background-sessions-followups

Conversation

@oonid

@oonid oonid commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Follow-ups from #16, planned in docs/plans/background-sessions-followups.md.

What changes

  • End's drain window is 10 s (was 3 s). After SIGTERM and SIGCONT, End waits up to 10 s for the session's programs to exit, and stops waiting as soon as they have. Remove's wait and the "Ending the previous session…" give-up are derived from End's worst case (17.5 s, giving a 20 s timeout), defined in one place in ManagedSession, instead of separate fixed 10 s values.
  • Install, remove and test run in an app-scoped runner (DistroOperations, owned by App like SessionManager). Before, they ran in the list screen's own coroutine scope with their state in remember. A rotation or a dark-mode switch therefore lost the output and the busy state while pr-cli kept running, and a second operation could start on top. Now the list only renders the runner's state, one operation runs at a time, and the custom-image fields use rememberSaveable.
    • New user-visible text: "Stopping the session…" while Remove waits for a running session to end.
    • Other texts, pr-cli commands and arguments are unchanged. runDistroCommand moved to PrCliCommand.
  • The output sheet stays up while an operation runs. Material3 hides the sheet before it asks to dismiss. A refused swipe used to leave an invisible sheet that swallowed every touch until the operation ended. The sheet state now refuses to hide while an operation is running.
  • The service's start/stop decisions are a plain class (ServiceState), which SessionService executes. The running notification is never titled "0 sessions running"; a login that fails at once now shows "Stopping…" for the moment it is visible.
  • /proc scanning goes through a seam (ProcFs / ProotProcesses), with the real /proc as the default. The existence check is kept exactly as before (exists, a sixth read beside the five the plan listed).

Verification

  • Unit tests: 105 passed (DistroOperations 15, ManagedSession 24, SessionManager 29, ProcStat 11, ProotProcesses 13, ServiceState 11, SessionNotifications 2). The new tests were written first and seen failing. The scan-race tests and the worst-case timing test are checked by mutation: they fail when their guard or a formula term is removed.
  • Device checks (Android 16):
    • End with a program that needs 5 s to drain on SIGTERM: its handler finished, and the session ended at 5.3 s. The old 3 s cap would have cut it off.
    • End with a program that ignores SIGTERM: the session ended at 12.2 s, within the 17.5 s worst case.
    • A login that fails at once: "Session ended (exit 1)", no crash, and no "0 sessions running".
    • Test: swiping the sheet down while it ran was refused. A rotation and two dark-mode switches during the run kept the sheet and its output, and it finished with 55/55 passed.
    • Custom install: the text fields kept their contents across a rotation, and a rotation during the install kept the sheet and output.
    • Remove of a container with a running session whose program needs 5 s to drain: "Stopping the rmtest session…", the program drained cleanly, a rotation during the wait changed nothing, then the rootfs was wiped. The list refreshed.

Not covered

  • Process death still loses a running list operation (documented): no foreground service covers it.
  • The scan-race tests depend on the order in which the scan lists /proc. A refactor that changes it makes them fail loudly, not pass silently.
  • A race that predates this PR: Remove checks for a running session once, so a session reopened in the moment between its end and pr-cli remove (for example from a terminal in split screen waiting on "Ending…") would run under the removal. Worth its own follow-up.
  • SessionService and PrCliCommand themselves (the Android calls) remain device-verified only. Their decisions and line handling are JVM-tested.

oonid added 11 commits October 18, 2026 15:06
Decisions from the PR #16 follow-up list: a 10 s drain window with derived Remove and Ending timeouts, JVM tests through small seams for /proc scanning and the service, an app-scoped runner for install, remove and test, and no "0 sessions running" notification.
SessionService now executes the actions a plain ServiceState returns
for each start and running-list change: start foreground first, notify,
wake lock, and stop with the last start id once foreground and empty.
JVM tests cover the immediate stop, a start arriving while stopping, a
conflated empty list and the wake lock. A start that finds no session
titles its notification "Stopping..." instead of "0 sessions running".
ProotProcesses holds ProotPty's scan (TracerPid tracees, the children
files of every task, the stat-scan fallback, alive) and reads through
a small ProcFs interface with the real /proc as the default. JVM tests
with a fake process table cover tracee selection, another proot's
tracees, a daemon reparented to init, the descendant-walk fallback, a
pid vanishing mid-scan, zombies, and the force-kill order.
End now waits up to 10 s for the signalled programs to exit, still
stopping as soon as they have. ManagedSession defines End's worst case
(17.5 s) and the timeout derived from it (20 s), which Remove's wait and
the "Ending the previous session" give-up now use instead of fixed 10 s.
ProcFs gains exists(pid), so a pid whose status is unreadable while
/proc/<pid> still exists forces the descendant walk again, as before
the seam. The fake process table now vanishes a pid after a chosen
listing and records reads made after it vanished, so one test removes
a pid between the status listing and its read and another during the
fallback walk. The runningTitle tests move to SessionNotificationsTest.
The slowest-End test now uses 400 ms steps and expects the sum of the
worst case's steps, without the scan margin, so dropping any term from
endWorstCaseMs fails it. The 4 s drain-window test is dropped: early
stopping and the 10 s default are covered elsewhere.
Every step of the slowest End is now 1 s, the kill-exit wait included,
which becomes a constructor parameter like the other waits, and the
upper slack is 800 ms. The slack stays below every step, so a step
missing from endWorstCaseMs still fails the test.
Install, remove and test from the list ran in the screen's coroutine
scope with their output in remember state, so rotating or switching
dark mode dropped the output, the busy state and Remove's session
wait while pr-cli kept running, and a second operation could start.

DistroOperations, owned by App, runs one pr-cli operation at a time
through a DistroCommand seam (PrCliCommand, the former
runDistroCommand) and exposes it as a StateFlow. Remove ends a
running session first and waits up to END_TIMEOUT_MS. MainActivity
only renders the state; the custom image fields use rememberSaveable.
The design and architecture docs describe DistroOperations, how the
list renders it, Remove's stopping line, and its JVM tests.
Material3 settles a swiped sheet to Hidden before it calls
onDismissRequest, so refusing dismiss() there left a hidden sheet
whose popup swallowed every touch until the operation finished. The
sheet state now refuses Hidden while the operation runs, which also
covers scrim taps and Back; once it has finished they dismiss and
refresh the list as before.
PrCliCommand closes the process output when the read ends. The worst-case End test now allows the whole derived worst case, scan margin included, so a loaded CI host does not fail it; dropping a step from the formula still fails it. The timeout test no longer restates the constants. The docs say a list operation does not survive the process being killed, and integration-tests.md names the runner instead of the removed runDistroCommand.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant