Skip to content

Conversation

@alexcrichton
Copy link
Member

This commit fixes a panic in the ParkingSpot implementation where an
unpark signal may not get acknowledged when a waiter times out,
causing the waiter to remove itself from the internal map but panic
thinking that it missed an unpark signal.

The fix in this commit is to consume unpark signals when a timeout
happens. This can lead to another possible race I've detailed in the
comments which I believe is allowed by the specification of park/unpark
in wasm.

Use `std::thread::scope` to keep everything local to just the tests.
This commit fixes a panic in the `ParkingSpot` implementation where an
`unpark` signal may not get acknowledged when a waiter times out,
causing the waiter to remove itself from the internal map but panic
thinking that it missed an unpark signal.

The fix in this commit is to consume unpark signals when a timeout
happens. This can lead to another possible race I've detailed in the
comments which I believe is allowed by the specification of park/unpark
in wasm.
@alexcrichton
Copy link
Member Author

This was uncovered by this failing test run

@alexcrichton
Copy link
Member Author

We also didn't find this earlier since nothing in the test suite presumably exercises wait-with-timeout until #5858 was merged.

Copy link
Member

@abrown abrown left a comment

Choose a reason for hiding this comment

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

Good find! Glad I actually worked on #5858; I'm looking at bringing in the wasi-threads spec tests as well which might find more.

@alexcrichton alexcrichton added this pull request to the merge queue Feb 23, 2023
Merged via the queue into bytecodealliance:main with commit f91640f Feb 24, 2023
@alexcrichton alexcrichton deleted the fix-unpark-panic branch February 24, 2023 00:44
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