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

clarify RW lock's priority gotcha #82596

Merged
merged 2 commits into from
Feb 28, 2021
Merged

clarify RW lock's priority gotcha #82596

merged 2 commits into from
Feb 28, 2021

Conversation

matklad
Copy link
Member

@matklad matklad commented Feb 27, 2021

In particular, the following program works on Linux, but deadlocks on
mac:

    use std::{
        sync::{Arc, RwLock},
        thread,
        time::Duration,
    };

    fn main() {
        let lock = Arc::new(RwLock::new(()));

        let r1 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r1/1");
                sleep(1000);

                let _rg = lock.read();
                eprintln!("r1/2");

                sleep(5000);
            }
        });
        sleep(100);
        let w = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _wg = lock.write();
                eprintln!("w");
            }
        });
        sleep(100);
        let r2 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r2");
                sleep(2000);
            }
        });

        r1.join().unwrap();
        r2.join().unwrap();
        w.join().unwrap();
    }

    fn sleep(ms: u64) {
        std::thread::sleep(Duration::from_millis(ms))
    }

Context: I was completely mystified by a my CI deadlocking on mac (here), until @azdavis debugged the issue. See a stand-alone reproduciton here: matklad/xshell#15

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 27, 2021
In particular, the following program works on Linux, but deadlocks on
mac:

    use std::{
        sync::{Arc, RwLock},
        thread,
        time::Duration,
    };

    fn main() {
        let lock = Arc::new(RwLock::new(()));

        let r1 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r1/1");
                sleep(1000);

                let _rg = lock.read();
                eprintln!("r1/2");

                sleep(5000);
            }
        });
        sleep(100);
        let w = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _wg = lock.write();
                eprintln!("w");
            }
        });
        sleep(100);
        let r2 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r2");
                sleep(2000);
            }
        });

        r1.join().unwrap();
        r2.join().unwrap();
        w.join().unwrap();
    }

    fn sleep(ms: u64) {
        std::thread::sleep(Duration::from_millis(ms))
    }
Co-authored-by: Steven Fackler <sfackler@gmail.com>
@sfackler
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 27, 2021

📌 Commit 261c952 has been approved by sfackler

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2021
@ojeda
Copy link
Contributor

ojeda commented Feb 27, 2021

Perhaps add a time-threads diagram with the deadlock example, e.g.:

// Thread 1             |  // Thread 2
let _rg = lock.read();  |
                        |  // will block
                        |  let _wg = lock.write();
// may deadlock         |
let _rg = lock.read();  |

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 27, 2021
clarify RW lock's priority gotcha

In particular, the following program works on Linux, but deadlocks on
mac:

```rust
    use std::{
        sync::{Arc, RwLock},
        thread,
        time::Duration,
    };

    fn main() {
        let lock = Arc::new(RwLock::new(()));

        let r1 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r1/1");
                sleep(1000);

                let _rg = lock.read();
                eprintln!("r1/2");

                sleep(5000);
            }
        });
        sleep(100);
        let w = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _wg = lock.write();
                eprintln!("w");
            }
        });
        sleep(100);
        let r2 = thread::spawn({
            let lock = Arc::clone(&lock);
            move || {
                let _rg = lock.read();
                eprintln!("r2");
                sleep(2000);
            }
        });

        r1.join().unwrap();
        r2.join().unwrap();
        w.join().unwrap();
    }

    fn sleep(ms: u64) {
        std::thread::sleep(Duration::from_millis(ms))
    }
```

Context: I was completely mystified by a my CI deadlocking on mac ([here](matklad/xshell#7)), until `@azdavis` debugged the issue. See a stand-alone reproduciton here: matklad/xshell#15
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#81856 (Suggest character encoding is incorrect when encountering random null bytes)
 - rust-lang#82395 (Add missing "see its documentation for more" stdio)
 - rust-lang#82401 (Remove a redundant macro)
 - rust-lang#82498 (Use log level to control partitioning debug output)
 - rust-lang#82534 (Link crtbegin/crtend on musl to terminate .eh_frame)
 - rust-lang#82537 (Update measureme dependency to the latest version)
 - rust-lang#82561 (doc: cube root, not cubic root)
 - rust-lang#82563 (Fix intra-doc handling of `Self` in enum)
 - rust-lang#82584 (Add ARIA role to sidebar toggle in Rustdoc)
 - rust-lang#82596 (clarify RW lock's priority gotcha)
 - rust-lang#82607 (Add a getter for Frame.loc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e38b3eb into rust-lang:master Feb 28, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 28, 2021
@matklad matklad deleted the rwlock branch February 28, 2021 07:36
ojeda added a commit to ojeda/rust that referenced this pull request Mar 5, 2021
Suggested in rust-lang#82596 but it was
a bit too late.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2021
…Titor

RWLock: Add deadlock example

Suggested in rust-lang#82596 but it was a bit too late.

`@matklad` `@azdavis` `@sfackler`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants