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.

Sign up for GitHub

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

Jump to bottom

Eagerly reset shared state in async_rw_mutex #677

Merged
merged 1 commit into from
May 10, 2023
Merged

Eagerly reset shared state in async_rw_mutex #677

merged 1 commit into from
May 10, 2023

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented May 8, 2023 •
edited
Loading

Eagerly reset the shared state held by the operation state to allow continuations to run as early as possible. This fixes a deadlock that occurred in the added test case.

@msimberg msimberg requested a review from aurianer as a code owner May 8, 2023 10:06
@msimberg msimberg self-assigned this May 8, 2023
@msimberg msimberg requested a review from biddisco as a code owner May 8, 2023 10:06
@msimberg msimberg mentioned this pull request May 8, 2023
Merged
2 tasks
@msimberg msimberg added this to the 0.16.0 milestone May 8, 2023
@msimberg
Copy link
Contributor Author

msimberg commented May 8, 2023

bors try

bors bot added a commit that referenced this pull request May 8, 2023
@bors
Try #677:
f2c1a5d
@bors
Copy link
Contributor

bors bot commented May 8, 2023

try

Build failed:

aurianer
aurianer approved these changes May 8, 2023
View reviewed changes
Copy link
Contributor

@aurianer aurianer left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@msimberg msimberg force-pushed the async-rw-mutex-eagerly-reset-state branch from 6c1e5cc to 76f8e93 Compare May 9, 2023 08:40
@msimberg
Copy link
Contributor Author

msimberg commented May 9, 2023

I've split out the spinlock change into a separate PR: #679.

@msimberg msimberg changed the title Small fixes to async_rw_mutex Eagerly reset shared state in async_rw_mutex May 9, 2023
@msimberg
Copy link
Contributor Author

msimberg commented May 9, 2023

bors try

bors bot added a commit that referenced this pull request May 9, 2023
@bors
Try #677:
feed0b1
@bors
Copy link
Contributor

bors bot commented May 9, 2023

try

Build failed:

@msimberg
Copy link
Contributor Author

msimberg commented May 9, 2023

bors try

bors bot added a commit that referenced this pull request May 9, 2023
@bors
Try #677:
a16a3e3
@msimberg
Copy link
Contributor Author

msimberg commented May 9, 2023

Note that I have bumped the stdexec commit used for the nvhpc configuration in CI to the one from #678. I have not changed anything else so #678 is still needed. However, nvc++ presumably miscompiling the test with the older commit (the async_rw_mutex test passed with all other configurations, segfaulted with nvc++). The reason is probably no_unique_address which seems to be buggy with nvc++.

@bors
Copy link
Contributor

bors bot commented May 9, 2023

try

Build failed:

@msimberg
Copy link
Contributor Author

msimberg commented May 9, 2023

This does actually need #678 to go in first because of some changes in stdexec. The async_rw_mutex test passes now with nvc++ though.

@msimberg msimberg force-pushed the async-rw-mutex-eagerly-reset-state branch from 169beb8 to 5bbc6f3 Compare May 10, 2023 15:27
@msimberg
Copy link
Contributor Author

I've removed the stdexec changes from this PR and simply disabled the new test for now with nvhpc+stdexec. It needs to be reenabled again in #678.

bors merge

bors bot added a commit that referenced this pull request May 10, 2023
@bors @msimberg
Merge #677
1bb7fbc 
677: Eagerly reset shared state in `async_rw_mutex` r=msimberg a=msimberg

Eagerly reset the shared state held by the operation state to allow continuations to run as early as possible. This fixes a deadlock that occurred in the added test case.

Co-authored-by: Mikael Simberg <mikael.simberg@iki.fi>
@bors
Copy link
Contributor

bors bot commented May 10, 2023

Build failed:

@msimberg msimberg merged commit a7904d8 into main May 10, 2023
@bors bors bot deleted the async-rw-mutex-eagerly-reset-state branch May 10, 2023 17:02
@msimberg msimberg modified the milestones: 0.16.0, 0.15.1 May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aurianer aurianer aurianer approved these changes

@biddisco biddisco Awaiting requested review from biddisco biddisco is a code owner

Assignees

@msimberg msimberg

Labels
None yet
Projects
pika planning board
Archived in project
Milestone
0.15.1
Development

Successfully merging this pull request may close these issues.
2 participants
@msimberg @aurianer