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

RwLock can sometimes deadlock on Windows with only read locks holding or waiting for the lock #121949

Closed
SkiFire13 opened this issue Mar 3, 2024 · 1 comment · Fixed by #121956
Assignees
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@SkiFire13
Copy link
Contributor

SkiFire13 commented Mar 3, 2024

I tried this code:

use std::sync::{Barrier, RwLock};

fn main() {
    for i in 0.. {
        let lock = RwLock::new(());

        let barrier = Barrier::new(5);
        let write_guard = lock.write();

        std::thread::scope(|s| {
            for _ in 0..5 {
                s.spawn(|| {
                    let read_guard = lock.read();
                    barrier.wait();
                    drop(read_guard);
                });
            }

            drop(write_guard);
        });

        println!("{i}");
    }
}

I expected to see this happen: the program continues to print increasing numbers, never stopping.

Instead, this happened: it stop after a non-deterministic amount of numbers printed, usually less than 1000.

The example is adapted from this C++ thread https://www.reddit.com/r/cpp/comments/1b55686/maybe_possible_bug_in_stdshared_mutex_on_windows/
According to some comments it seems to be a bug in Windows' SRWLOCK which Rust appears to use for RwLock.

Opened as requested on zulip https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/SRWLOCK.20bug/near/424539022

Meta

rustc --version --verbose:

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-pc-windows-msvc
release: 1.76.0
LLVM version: 17.0.6
@SkiFire13 SkiFire13 added the C-bug Category: This is a bug. label Mar 3, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 3, 2024
@ChrisDenton ChrisDenton added O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 3, 2024
@ChrisDenton
Copy link
Contributor

As it happens, I was investigating switching to the WaitOnAddress APIs. This provides a very good motivation for me to prioritise it.

@rustbot claim

@bors bors closed this as completed in 3314d5c Mar 6, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 16, 2024
Use queue-based `RwLock` on more platforms

This switches over Windows 7, SGX and Xous to the queue-based `RwLock` implementation added in rust-lang#110211, thereby fixing rust-lang#121949 for Windows 7 and partially resolving rust-lang#114581 on SGX. TEEOS can't currently be switched because it doesn't have a good thread parking implementation.

CC `@roblabla` `@raoulstrackx` `@xobs` Could you help me test this, please?
r? `@ChrisDenton` the Windows stuff should be familiar to you
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 16, 2024
Rollup merge of rust-lang#123811 - joboet:queue_em_up, r=ChrisDenton

Use queue-based `RwLock` on more platforms

This switches over Windows 7, SGX and Xous to the queue-based `RwLock` implementation added in rust-lang#110211, thereby fixing rust-lang#121949 for Windows 7 and partially resolving rust-lang#114581 on SGX. TEEOS can't currently be switched because it doesn't have a good thread parking implementation.

CC `@roblabla` `@raoulstrackx` `@xobs` Could you help me test this, please?
r? `@ChrisDenton` the Windows stuff should be familiar to you
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants