这是indexloc提供的服务,不要输入任何密码
Skip to content

feat(process): differentiate between child interruption and killing #10027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 25, 2025

Conversation

chris-olszewski
Copy link
Member

Description

This is primarily a code move of the ProcessManager out of the turborepo-lib crate. We also now differ between a child shutting down in response to a SIGINT vs us killing the child.

Future PR will be changing how we treat each of these exit outcomes.

Testing Instructions

Existing unit tests 👀

Copy link

vercel bot commented Feb 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 25, 2025 8:56pm

@chris-olszewski chris-olszewski changed the title chore(process): feat(process): differentiate between child interruption and killing Feb 24, 2025
@@ -0,0 +1,27 @@
[package]
name = "turborepo-process"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it

@chris-olszewski chris-olszewski force-pushed the olszewski/chore_process_manager branch from e1a89d9 to 767519c Compare February 25, 2025 20:55
@chris-olszewski chris-olszewski merged commit bc351c4 into main Feb 25, 2025
39 checks passed
@chris-olszewski chris-olszewski deleted the olszewski/chore_process_manager branch February 25, 2025 21:28
chris-olszewski added a commit that referenced this pull request Feb 26, 2025
…rruption and killing" (#10046)

Reverts #10027

`child::test::test_graceful_shutdown::false_expects` is currently flakey
as we race `child.wait()` calls:
- one knows we sent the signal so when the child exits we get a
`Interrupt` exit
- one does not know we sent a signal, so when the child exits from a
signal we get a `KilledExternal` as we think something else on the
system killed it

Will remerge once test has race condition removed.
chris-olszewski added a commit that referenced this pull request Mar 12, 2025
)

### 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:
> This is primarily a code move of the `ProcessManager` out of the
`turborepo-lib` crate. We also now differ between a child shutting down
in response to a `SIGINT` vs us killing the child.

> Future PR will be changing how we treat each of these exit outcomes.

The additional commits in this PR:
- Fixed a race condition between between the 2 `wait`s performed by the
`ChildStateManager` where one was called by the task the sent a shutdown
to the child and the other was the default `wait`. If the latter won,
then it would appear another entity killed the child even when it was
really us.
- Changed the return value of `ShutdownStyle::process` to avoid
returning an impossible state
 - Remove unused `ChildState::Exited` field which was never read
- Removed shared state between the child handle and the child state
manager
 - Simplified child methods

The 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:
- What does it mean if the manager is still listening for commands, but
the state shows the child as exited
- What does it mean if the manager channel is closed, but the state
shows the child is still running
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 500` to attempt
to flush out any races.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants