ADFA-4458 fix(proot): wait on the local process, not the shared currentProcess field#64
Merged
Conversation
…ntProcess field executeInContainer/startInteractiveShell stored the process in the shared instance field currentProcess and called waitFor() on the field. A concurrent proot call on the same engine instance (e.g. fetchLocalVarsFromPRoot / verifyInstallationState on fragment lifecycle) reassigned the field, so a long install's waitFor returned on the wrong (short-lived) process and onProcessExit fired early — the launch button reported completion while runrole was still running. Capture the process in a local variable per call; keep currentProcess only so killProcess targets the latest. Ref: ADFA-4435
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.
ADFA-4458 — install button reports completion before runrole finishes
Cause (concurrency)
PRootEngine.executeInContainer/startInteractiveShellstored the running process in the shared instance fieldcurrentProcessand calledwaitFor()on that field. The install flow and other proot calls (fetchLocalVarsFromPRoot,verifyInstallationState, fired on DeployFragment lifecycle e.g. when switching tabs) use the same shared engine (host.prootEngine()). A concurrent proot call reassignscurrentProcess, so a long install'swaitFor()returns on the wrong (short-lived) process →onProcessExitfires early → the launch button reports "finished" whilerunroleis still running (Ansible workers orphaned, visible in the server log).Pre-existing/latent — not introduced by the ADFA-4434 carve (verified:
PRootEngineuntouched; single shared engine instance both before and after). Distinct root cause from ADFA-4435.Fix
Capture the process in a local variable per call and stream/
waitFor()on the local; keepcurrentProcess = proconly sokillProcess()targets the latest. Each execution now waits on its own process; concurrent proot calls no longer race.Test on device
Start a module install, switch to Usage to watch the Server Log, switch back: the launch button must stay in "installing" until the role actually completes (no premature "finished").
Note (optional hardening, not in this PR)
Could also avoid firing
fetchLocalVars/verifywhileisBatchInstalling, or serialize proot calls — tracked separately if wanted.Relates to: ADFA-4435