feat(process): distinguish between signals used to kill children #10049
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
Contributor
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
chris-olszewski
commented
Feb 28, 2025
| child.stop().await.ok(); | ||
| }; | ||
|
|
||
| let (_, code) = join! { |
Contributor
Author
There was a problem hiding this comment.
With the lack of state, we no longer need to drive both futures to completion at the same time. Sending the stop command will complete:
- Once the child manager reads the command from the channel
- If the child manager exits and closes the command channel
After either of these we can wait for the exit code to be reported:
- If the stop command was read, then the child manager will update the exit status based on the outcome of issuing the command
- If the stop command was dropped, then the child manager has exited and will have dropped the sender for the exit status channel. If the sender for a watch channel is dropped then
changed()will exit.
NicholasLYang
approved these changes
Mar 12, 2025
b65db46 to
1d53a8c
Compare
1 task
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.
Description
This is a redo of #10027 with some additional refactoring to reduce the complexity of this code. These changes should lessens the possibilities of deadlock or race conditions.
I highly recommend viewing each commit individually.
From original PR:
The additional commits in this PR:
waits performed by theChildStateManagerwhere one was called by the task the sent a shutdown to the child and the other was the defaultwait. If the latter won, then it would appear another entity killed the child even when it was really us.ShutdownStyle::processto avoid returning an impossible stateChildState::Exitedfield which was never readThe removal of the shared state is possible since we already have the exit channel shared between the handles and the manager. Using both a channel and state was error prone as it lead to the following possibilities:
Instead we only use the command channel being open/closed as an indication of if the child is running. Once the manager sees the child exit, it will send the exit status via the channel and exit resulting in the channel being closed.
Testing Instructions
Existing test suite, ran with
hyperfine 'cargo test' -r 500to attempt to flush out any races.