Skip to content
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

block_current_task is impossible to use correctly #442

Closed
joboet opened this issue May 10, 2022 · 1 comment
Closed

block_current_task is impossible to use correctly #442

joboet opened this issue May 10, 2022 · 1 comment

Comments

@joboet
Copy link
Contributor

joboet commented May 10, 2022

The current API for task blocking is prone to timing bugs, leading to deadlocking. Consider the current RecMutex implementation:
https://github.com/hermitcore/libhermit-rs/blob/7c9327ef352d26ac0ba69c952530e9801b454cbd/src/synch/recmutex.rs#L26-L61
If a timer interrupt triggers a reschedule directly after calling block_current_task, the state spinlock will not be unlocked, therefore any task trying to wake the current one will deadlock, too. In general, almost all uses of block_current_task will need to do some operation after marking themselves as blocked, and are thus suffering from the same timing problem. Note however, that the current semaphore implementation prevents interrupts during its critical section, avoiding this problem. This is however not possible for user programs, such as the Parker in std.

@joboet
Copy link
Contributor Author

joboet commented Jun 21, 2022

Whoops, I didn't realise Hermit doesn't pre-empt tasks, so the race condition can't occur.

@joboet joboet closed this as completed Jun 21, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 24, 2022
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?
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 25, 2022
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?
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 25, 2022
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?
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

No branches or pull requests

1 participant