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

Do not free waker until safe #85

Merged
merged 1 commit into from
Apr 10, 2020
Merged

Do not free waker until safe #85

merged 1 commit into from
Apr 10, 2020

Conversation

jonhoo
Copy link
Owner

@jonhoo jonhoo commented Apr 10, 2020

It is possible for someone to try to wake a waiting thread while that
thread is spinning. In that case, the waiting thread may decide to free
its waker just before the waking thread tries to wake the waiting one,
resulting in a use-after-free.

This fixes that by deferring the freeing of the waker until the next
epoch.

Seems to fix #84.


This change is Reviewable

It is possible for someone to try to wake a waiting thread while that
thread is spinning. In that case, the waiting thread may decide to free
its waker just before the waking thread tries to wake the waiting one,
resulting in a use-after-free.

This fixes that by deferring the freeing of the waker until the next
epoch.

Seems to fix #84.
@jonhoo jonhoo requested a review from domenicquirl April 10, 2020 16:17
@codecov
Copy link

codecov bot commented Apr 10, 2020

Codecov Report

Merging #85 into master will increase coverage by 0.43%.
The diff coverage is 0.00%.

Impacted Files Coverage Δ
src/node.rs 80.07% <0.00%> (+0.52%) ⬆️
src/map.rs 85.27% <0.00%> (+0.36%) ⬆️
src/raw/mod.rs 89.32% <0.00%> (+1.94%) ⬆️

@jonhoo
Copy link
Owner Author

jonhoo commented Apr 10, 2020

It looks like this may also fix #83 somehow. At least I haven't been able to reproduce it with this fix. I'll keep running it for a bit.

@jonhoo
Copy link
Owner Author

jonhoo commented Apr 10, 2020

Nope, finally got another crash similar to #83, so this probably just fixes #84. It does seem to fix that though, so I'll merge 🎉

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.

Racy test failure: segfault in map::tree_bins::concurrent_tree_bin
2 participants