ADFA-4842: bring the server back after a module install - #295
Merged
Conversation
Deterministic fix for the post-module-install dead/blocked home. A runrole modifies system packages/config and must own the rootfs exclusively, so: - InstallService.runModuleQueue now runs 'pdsm stop' before the runroles, so a runrole never runs alongside a live server writing the same DBs/config (data-corruption risk; same exclusive-rootfs pattern as a fresh install). - LibraryActivity observes the module queue: while RUNNING it blocks the WHOLE UI behind the full-screen boot gate (every tab + nav) and shows the current module; on DONE it restarts the server. Because the server was explicitly stopped, it is unambiguously down at DONE, so the restart is deterministic (no stale-'alive' race that made the earlier cold-boot land on a still-'up' then-dropped home). - finishModuleQueue clears InstallGuard before posting DONE so the restart's canStartServer() (which requires !InstallGuard.inProgress) is not raced. Supersedes the cold-boot recreate in SetupProgressActivity.goHome (reverted here): goHome returns to the live Library normally; the queue observer owns the block + restart. Integrates the approach from the feat/ADFA-4842-module-mgmt branch onto current main (post #292).
…ry block The Library-side moduleBusy block/observer from the previous commit doesn't fit the current flow: module installs run on SetupProgressActivity (the index) with LibraryActivity STOPPED behind it, so the observer never saw RUNNING and never restarted the server on DONE (and the full-screen block sat behind the index). Correct it for the index-on-top flow: - Drop the LibraryActivity moduleBusy field, observer, and showModuleBusy/hideModuleBusy. The index already blocks the UI during the batch (it's the last barrier). - Re-apply the goHome cold-boot: a module batch returns to a FRESH LibraryActivity (CLEAR_TOP without SINGLE_TOP → onCreate) so its normal boot gate + autostart run. Kept from the previous commit (the actual fix): 'pdsm stop' before the runroles, which leaves the server deterministically DOWN — so the cold-boot's autostart reliably sees !alive and restarts it (this is why the earlier cold-boot alone failed: without the stop the server looked 'up' at DONE). InstallGuard is still cleared before DONE.
Testing showed the previous approach still landed on a dead home: the index declared the batch complete on runrole DONE and redirected while the server was still down, so Library woke up seconds later. Move the wait to the index, as it should be: - SetupProgressActivity now owns a ServerController (minimal Host). When a MODULE batch reaches DONE, it restarts the server and polls the REST core (RestReadiness.apiReady) showing "Starting services..."; completion + the redirect countdown are gated on the server actually answering. Only then does it go to an already-live Library (plain CLEAR_TOP + SINGLE_TOP reuse — no cold-boot recreate). - InstallService.runModuleQueue scopes 'pdsm stop' to batches that contain a real module; a maps-only batch skips it (maps coexists with a live server), so the wizard/Get-More maps flow is unchanged (no server bounce, no extra wait). The server proot is process-scoped, so the one the index starts survives into LibraryActivity; Library sees it alive and does not re-start it.
Bug: 'Run in background' (and Back) reappeared during the 'Starting services...' phase. prootActive() went false at runrole DONE, but that is exactly when the index restarts the server and waits — so the proot-only gate lifted early and the user could background out mid-restart, aborting it. Fix: a module session stays 'proot active' from its runroles THROUGH the post-DONE server restart, until the REST core answers (moduleServerUp) or a 45s safety timeout. This hides 'Run in background' for the whole module session (there is no backgrounding a proot install) and keeps Back trapped (snackbar → move-to-back) until the system is live. The timeout prevents trapping the user forever if the server never comes up (Library owns recovery).
…s' dots - Finish/redirect now returns to the Library (Home) tab, not the tab the install was launched from. goHome passes EXTRA_TAB=nav_library and LibraryActivity.onNewIntent honors it on the reused instance (Module management opens from Settings, so without this the user landed back on Settings). Not proot-specific — applies to any install completion. - The amber 'Starting services' line now animates an ellipsis (dots appear/disappear) so the wait never looks frozen, including the post-module server restart. Stopped in onPause.
The animated status dots were being re-declared per screen (LibraryActivity, and the copy I just added to SetupProgressActivity). Extract a small reusable util/EllipsisAnimator (attach to a TextView, start(base)/stop(), strips trailing dots, 0..3 cycle) and use it in the index instead of a private Handler/Runnable. LibraryActivity/TerminalController can migrate to it later (their variant uses a fixed-width monospace span) — left as a follow-up to keep this change focused.
…rver Code-review follow-up (#295). moduleServerUp was only set inside the noRest branch, so a module session that ever carried REST work would never trigger ensureServerUpForModules() — leaving prootActive() blocked forever (no Run-in-background, Back trapped) with the 45s timeout never armed. Kick ensureServerUpForModules() for ANY terminal module session and gate both completion branches on (!moduleShown || moduleServerUp), so the server restart + its safety timeout always run. Module batches are proot-only today; this just removes the latent trap.
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
A module (proot) install left the system's services down when it finished, so the
Library landed on a blocked/offline home. This stops the server cleanly before the
runroles and brings it back after, landing on a live system.
Changes
InstallService.runModuleQueue: runpdsm stopbefore the runroles so each runroleowns the rootfs exclusively — no runrole writing the same DBs/config as a live server
(the data-corruption risk). Also clears
InstallGuardbefore postingDONE.SetupProgressActivity.goHome: after a module batch, return to a freshLibraryActivity(CLEAR_TOP without SINGLE_TOP → onCreate) so its normal boot gate +autostart run. Because the server was explicitly stopped, the autostart reliably sees
it down and restarts it. Maps/REST sessions keep the server up and reuse the live
Library unchanged.
Why the cold-boot alone wasn't enough
Without the
pdsm stop, the server still looked "up" at the moment the batch finished,so the autostart skipped the restart. The deterministic stop is what makes it fire.
On-device verification
per-module progress; on finish the boot gate shows and the server comes back up on its
own to a live home (previous + new modules).
pdsm stop, reuse live Library.Supersedes
feat/ADFA-4842-module-mgmt(#255).