Wasmtime GitHub notifications bot (Dec 04 2023 at 17:58): Wasmtime GitHub notifications bot (Dec 04 2023 at 17:58):alexcrichton opened PR #7629 from alexcrichton:fix-wait-notify to bytecodealliance:main:
This commit rewrites and refactors the
ParkingSpotimplementation in Wasmtime. This is motivated by #7623 primarily which is something I haven't been able to reproduce but it doesn't look like a spurious issue. Additionally in reviewing the previous implementation I'm not sure if it was technically spec-compliant.Previously each parking spot was modeled with a single condition variable. All threads blocking on the same address would block on the same condition variable. When waking up N threads the condition variable would either use
notify_allornotify_oneN times. The part I wasn't so sure about is that each thread, when waking up, would "consume" a wakeup notification on the way out, going back to sleep if a notification wasn't available. This was intended to handle spurious wakeups from the OS condition variable in use. This could mean, however, that a spurious wakeup of one thread could consume a notification from another thread. This was documented already as a possibility and "probably ok" but my gut is that this behavior led to #7623, although I haven't been able to construct a trace that would lead the test here to deadlock.Out of caution, however, this commit rewrites the implementation to be similar to what V8 and SpiderMonkey are doing. Both of those implementations are using a linked list of waiters for threads that are blocked and then notifying N threads dequeues N threads to wake them up. This makes the semantics of knowing which thread is waken up easier to understand from an implementation point of view since each wakeup notification deterministically goes to a specified thread. The tricky part about this implementation is that a doubly-linked-list needs to be maintained within
ParkingSpotto handle this.While I was here I additionally refactored the interface of
ParkingSpotto more closely match the raw interface of WebAssembly. This is intended to scope the problem being solved more narrowly to what wasm needs rather than trying to solve a more general problem at the same time.Closes #7623
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Dec 04 2023 at 17:58):alexcrichton requested pchickey for a review on PR #7629.
Wasmtime GitHub notifications bot (Dec 04 2023 at 17:58):alexcrichton requested wasmtime-core-reviewers for a review on PR #7629.
Wasmtime GitHub notifications bot (Dec 04 2023 at 18:32):alexcrichton updated PR #7629.
Wasmtime GitHub notifications bot (Dec 05 2023 at 00:58):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Dec 05 2023 at 17:07):alexcrichton merged PR #7629.
Last updated: Dec 23 2024 at 12:05 UTC