-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
std: use an event-flag-based thread parker on SOLID #97140
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
I noticed this could be using a semaphore, which might be more efficient, as the system does not need to compare the bitflags... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following code deadlocks under this implementation:
let thread0 = std::thread::current();
std::thread::spawn(move || {
thread0.unpark();
});
std::thread::park_timeout(std::time::Duration::from_millis(50)); // <--- blocks indefinitely here
Reconstructed execution sequence:
st Thread 0 Thread 1
--------------------------------------------------------------------------
0 park_timeout:
-1 st.fetch_sub(1) // 0
twai_flg:
[yield]
unpark:
1 st.swap(1) // -1
set_flg:
[wake up thread 0]
[resume]
0 st.swap(0) // 1
wai_flg
[deadlock]
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memory orderings can be relaxed to Acquire
and Release
like the futex-based implementation.
The rest looks good to me.
Do we actually make any atomic ordering guarantees? It says so in the |
It doesn't appear to be documented explicitly anywhere, but the common use cases of |
@kawadakk Ah, I understand now. Thank you for all the help! |
Co-authored-by: Tomoaki Kawada <[email protected]>
ping @m-ou-se: I think all issues are fixed. |
@bors r+ |
📌 Commit caff723 has been approved by |
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API (which is unfortunately [broken](hermit-os/kernel#442), I think). ``@kawadakk`` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API (which is unfortunately [broken](hermit-os/kernel#442), I think). ```@kawadakk``` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API (which is unfortunately [broken](hermit-os/kernel#442), I think). ````@kawadakk```` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API). `@kawadakk` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API). ``@kawadakk`` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API). ```@kawadakk``` I assume you are the target maintainer? Could you test this for me?
std: use an event-flag-based thread parker on SOLID `Mutex` and `Condvar` are being replaced by more efficient implementations, which need thread parking themselves (see rust-lang#93740). Therefore, the generic `Parker` needs to be replaced on all platforms where the new lock implementation will be used, which, after rust-lang#96393, are SOLID, SGX and Hermit (more PRs coming soon). SOLID, conforming to the [μITRON specification](http://www.ertl.jp/ITRON/SPEC/FILE/mitron-400e.pdf), has event flags, which are a thread parking primitive very similar to `Parker`. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, this `Parker`, like the Windows parker, uses an extra atomic state variable. I future-proofed the code by wrapping the event flag in a `WaitFlag` structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API). ````@kawadakk```` I assume you are the target maintainer? Could you test this for me?
…askrgr Rollup of 11 pull requests Successful merges: - rust-lang#97140 (std: use an event-flag-based thread parker on SOLID) - rust-lang#97295 ([rustc_parse] Forbid `let`s in certain places) - rust-lang#97743 (make const_err show up in future breakage reports) - rust-lang#97908 (Stabilize NonZero* checked operations constness.) - rust-lang#98297 (Transform help popup into a pocket menu) - rust-lang#98428 (macros: use typed identifiers in diag and subdiag derive) - rust-lang#98528 (Respect --color when building rustbuild itself) - rust-lang#98535 (Add regression test for generic const in rustdoc) - rust-lang#98538 (Add a ui test for issue rust-lang#91883) - rust-lang#98540 (Add regression test for rust-lang#87558) - rust-lang#98541 (Update `std::alloc::System` doc example code style) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Mutex
andCondvar
are being replaced by more efficient implementations, which need thread parking themselves (see #93740). Therefore, the genericParker
needs to be replaced on all platforms where the new lock implementation will be used, which, after #96393, are SOLID, SGX and Hermit (more PRs coming soon).SOLID, conforming to the μITRON specification, has event flags, which are a thread parking primitive very similar to
Parker
. However, they do not make any atomic ordering guarantees (even though those can probably be assumed) and necessitate a system call even when the thread token is already available. Hence, thisParker
, like the Windows parker, uses an extra atomic state variable.I future-proofed the code by wrapping the event flag in a
WaitFlag
structure, as both SGX and Hermit can share the Parker implementation, they just have slightly different primitives (SGX uses signals and Hermit has a thread blocking API).@kawadakk I assume you are the target maintainer? Could you test this for me?