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

Implement condvars for Windows #2638

Merged
merged 3 commits into from
Nov 6, 2022
Merged

Implement condvars for Windows #2638

merged 3 commits into from
Nov 6, 2022

Conversation

beepster4096
Copy link
Contributor

Adds 3 shims for Windows: SleepConditionVariableSRW, WakeConditionVariable, WakeAllConditionVariable to add support for condvars (which fixes #2628).

Salvaged from what was removed from #2231

@beepster4096 beepster4096 mentioned this pull request Oct 30, 2022
Closed
4 tasks
@beepster4096
Copy link
Contributor Author

I think this panic is caused by a mistake in #2601.

oli-obk
oli-obk reviewed Oct 30, 2022
View reviewed changes
src/concurrency/sync.rs Outdated Show resolved Hide resolved
oli-obk
oli-obk reviewed Oct 30, 2022
View reviewed changes
src/shims/windows/sync.rs Outdated Show resolved Hide resolved
@beepster4096 beepster4096 mentioned this pull request Oct 31, 2022
Merged
@beepster4096
Copy link
Contributor Author

CI failure should be fixed by #2641.

RalfJung
RalfJung reviewed Oct 31, 2022
View reviewed changes
src/shims/windows/sync.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 31, 2022
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Generally looks good, thanks! Mostly stylistic comments.

src/shims/windows/sync.rs Outdated Show resolved Hide resolved
src/shims/windows/sync.rs Outdated Show resolved Hide resolved
src/shims/windows/sync.rs Outdated Show resolved Hide resolved
tests/pass/concurrency/windows_condvar_shared.rs Show resolved Hide resolved
tests/pass/concurrency/windows_condvar_shared.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Nov 2, 2022
View reviewed changes
src/concurrency/sync.rs Outdated Show resolved Hide resolved
bors added a commit that referenced this pull request Nov 4, 2022
@bors
Auto merge of #2641 - DrMeepster:init_once_acquire, r=RalfJung
9568d7e 
InitOnce: synchronize with completion when already complete

The completion of an InitOnce happens-before the threads waiting on it wake up. However, this is not the case for threads that call `InitOnceBeginInitialize` after the completion, leading to data races and outdated weak memory loads as observed in the CI for  #2638. This PR fixes this.
@RalfJung
Copy link
Member

RalfJung commented Nov 4, 2022

I think this panic is caused by a mistake in #2601.

That PR landed, so please rebase this one. :)

@beepster4096
Copy link
Contributor Author

The rebase is done. CI's finally passing :)

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2022

Great. :) Please squash the commits a little, then we can land this.

@beepster4096
Copy link
Contributor Author

Squashed.

@RalfJung
Copy link
Member

RalfJung commented Nov 6, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 6, 2022

📌 Commit 958ca31 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 6, 2022

⌛ Testing commit 958ca31 with merge 32c90ff...

@bors