fix(supervisor): serialize child registration with reaping - #3142
Conversation
3a53122 to
f931959
Compare
There was a problem hiding this comment.
While reviewing the PR, I had an agent author a failing unit test that is fixed by the changes here. (This was verified locally and should not affect CI).
That means that I've updated this to the following stack:
test(supervisor): reproduce fast child reaping raceis intentionally red when checked out before the next commit. The following fix changes it to use the atomic registration API, so the history documents both the original failure and its resolution. The PR tip remains green.- The PR commit.
test(supervisor): cover child registration reaper raceprovides the durable deterministic interleaving check for the new locking invariant.
In applying the PR commit, I had to make changes to the test, which triggered a deeper investigation. It seems as if clients have to be sure to switch from spawn to spawn_registered. This seems to indicate that callers may need to decide between two different implementations and may select the wrong one. I thus created #3154 to track a follow-up to reduce the likelihood of this happening.
|
Label |
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com> Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
f931959 to
0d3ac48
Compare
|
Follow-up: #3156 is stacked on this PR and makes managed child lifecycle ownership structural across the canonical-process and SSH paths. It should be reviewed/merged after this PR. |
Summary
Fix a supervisor child-reaping race exposed by the fast detached-main coverage in #3128. A child that exited between
spawn()and managed-PID registration could be consumed by the orphan reaper, causing the canonical wait to fail withECHILDinstead of reporting the workload's real exit code.Related Issue
No issue required: this is a localized process-supervision race fix exposed by #3128.
Changes
waitpid.Testing
mise run pre-commitpassesmise run testpasses (withOPENSHELL_SYSTEM_GATEWAY_DIRpointed at an empty directory to isolate the host's installed gateway configuration)cargo test -p openshell-supervisor-processpasses (257 passed, 2 ignored)Checklist