fix(herdr): detach Linux remote doctor server start - #1
Merged
Merged
Conversation
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
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
fm-remote-doctor --fixcan return after readinesssetsidPGID transition before claiming process-group ownershipRoot cause
On Linux,
herdr server --session fm-remoteremains in the foreground. The previous nested background subshell inherited the command-substitution result pipe used by the doctor. Even after the shell running the start expression returned, the foreground Herdr process kept that pipe open, so command substitution and thereforefm-remote-doctor --fixdid not return.The start child now closes inherited descriptors above standard input, output, and error before
exec, while all three standard streams are redirected explicitly. Readiness still comes from the real status probe rather than a sleep.The first fork CI run exposed a separate race in the new test harness:
$!can become visible before the child completesexec setsid, so one immediatepssnapshot can still report the inherited PGID. A Linux ARM64 characterization observed this twice in 500 starts. The watchdog now polls the actual PGID transition within a one-second bound before using the group for lifecycle assertions or cleanup.Validation
tests/fm-remote-doctor.test.shruns passed after the PGID synchronization fix, including ready return and never-ready cleanup casestests/fm-remote-doctor.test.shpassedtests/fm-backend-herdr.test.shpassedbin/fm-lint.shpassed with ShellCheck 0.11.0bin/fm-doc-audience-check.shpassedgit diff --check origin/main..HEADpassedThe regression fails on the original implementation because the ready foreground server retains the result pipe. It also verifies that a never-ready direct child is bounded and reaped. No live
defaultHerdr session or Oracle host was touched.Pipeline
Updates from git push no-mistakes
Pipeline run
01M00PYZXYACX45CFVA0G279M3validated and pushed exact head934889bcf74123dce271daf0391f8a1693150832:no-mistakes axi run --skip test; the independent validation evidence is listed aboveThe run's PR target selected the upstream contribution route rather than the explicitly authorized fork-only route. That upstream PR was closed immediately without merge. This existing fork PR remains the authorized delivery surface and its own required CI is the acceptance gate.