-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Task killing, linked failure, and exit code propagation in the new runtime. #7858
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
Conversation
|
It looks to me that because of this line there is an additional CAS on every deschedule now. Is that correct? So receives are now two atomics? |
|
Same for send. A send that needs to wake another task is two atomics. |
|
Oh, but I guess that's where unkillable tasks come in. An indestructable task is unkillable and doesn't do the extra atomics. |
|
I'm r+ing this because I want to get the tests passing and the new runtime finished, but this model is too complicated. Now we have three different ways that tasks communicate failure where before we had two. You told me you were going to remove the complexity of supervision and that you were going to combine 'watched' and 'linked', and you did not. We're going to revisit this in the future. |
|
Here's a summary of my current concerns. Complexity The interface has a lot of options that it's not clear we need (linked, supervised, watched). I don't know that these are the semantics we want to commit to, nor that they can be explained easily. The addition of 'watched' mode makes the semantics more awkward. Bookkeeping Maintaining task groups requires a lot of book keeping, imposing several synchronizing operations on task spawning and termination. This bookkeeping involves taking a lock, and because by default there is a single task group, there is effectively a global lock on spawn/terminate. Interruptable blocking and I/O Even I/O is interruptable, but that means that all I/O will need to do thread synchronization. Shutdown This linkage model doesn't do anything to enable graceful runtime shutdown, and will require further bookkeeping to figure out when to tear down the schedulers. |
… blocking fastpath.
…tioning to newsched killing.
|
Whoops. Nasty memory leak I forgot to write a case for in port's destructor. Thanks to valgrind and having enough tests that I caught it. |
|
Though... I thought the runtime used to crash if the process exited with any leaked exchange allocations (i.e., not require valgrind to notice)? It doesn't seem to have done so here. |
Some notes about the commits. Exit code propagation commits: * ```Reimplement unwrap()``` has the same old code from ```arc::unwrap``` ported to use modern atomic types and finally (it's considerably nicer this way) * ```Add try_unwrap()``` has some new slightly-tricky (but pretty simple) concurrency primitive code * ```Add KillHandle``` and ```Add kill::Death``` are the bulk of the logic. Task killing commits: * ```Implement KillHandle::kill() and friends```, ```Do a task-killed check```, and ```Add BlockedTask``` implement the killing logic; * ```Change the HOF context switchers``` turns said logic on Linked failure commits: * ```Replace *rust_task ptrs``` adapts the taskgroup code to work for both runtimes * ```Enable taskgroup code``` does what it says on the tin. r? @brson
[`swap`] lints now check if there is `no_std` or `no_core` attribute Closes rust-lang#7858 changelog: [`swap`] lints now check if there is `no_std` or `no_core` attribute
Some notes about the commits.
Exit code propagation commits:
Reimplement unwrap()has the same old code fromarc::unwrapported to use modern atomic types and finally (it's considerably nicer this way)Add try_unwrap()has some new slightly-tricky (but pretty simple) concurrency primitive codeAdd KillHandleandAdd kill::Deathare the bulk of the logic.Task killing commits:
Implement KillHandle::kill() and friends,Do a task-killed check, andAdd BlockedTaskimplement the killing logic;Change the HOF context switchersturns said logic onLinked failure commits:
Replace *rust_task ptrsadapts the taskgroup code to work for both runtimesEnable taskgroup codedoes what it says on the tin.r? @brson