fix: use setsid to detach supervisor from branch child process group#237
Merged
fix: use setsid to detach supervisor from branch child process group#237
Conversation
SIGKILL to the branch daemon was propagating to the supervisor. Fix: launch supervisor with setsid so it runs in a new session, and use set +e around wait so non-zero exits don't abort the loop. Tested: SIGKILL branch → supervisor detects exit code 137 → restarts in 5s.
tps-kern
approved these changes
Mar 13, 2026
tps-kern
left a comment
tps-kern
left a comment
There was a problem hiding this comment.
Architecture verified. Using setsid to detach the process group is the correct Linux primitive to prevent signal propagation from children to the supervisor. Suspending set -e around the wait trap correctly captures the non-zero exit code of a crashed daemon without killing the supervisor script itself.
Collaborator
Author
|
Superseded by #238 which includes both the original supervisor scripts and the setsid fix. Closing. |
tps-sherlock
approved these changes
Mar 13, 2026
Contributor
tps-sherlock
left a comment
tps-sherlock
left a comment
There was a problem hiding this comment.
Security review complete.
The changes correctly prevent process group signaling (setsid) and errexit failures (set +e around wait) from killing the supervisor.
No security implications found. Approved.
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
4 participants
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.
Problem
After #235 merged, live testing showed SIGKILL to the branch daemon was killing the supervisor too. Root cause:
nohup ... &doesn't fully detach from the process group — SIGKILL propagated up.Fix
setup-branch-service.sh: launch supervisor withsetsidso it runs in a new session, isolated from the branch daemon's process groupbranch-supervisor.sh: wrapwaitwithset +e/set -eso non-zero exit codes (e.g. 137 from SIGKILL) don't abort the scriptTested
Branch restarts within 5s. Supervisor survives. ✅