Amanieu / parking_lot

Compact and efficient synchronization primitives for Rust. Also provides an API for creating custom synchronization primitives.
Apache License 2.0
2.77k stars 217 forks source link

`ReentrantMutex::bump()` probably needs to adjust the `lock_count` #389

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

I've found that ReentrantMutex::bump() can result in a panic!() in the thread that it hands off to when that thread calls lock(). A reproducible example of this is the following:

use parking_lot::ReentrantMutexGuard;
use parking_lot::ReentrantMutex;
use std::sync::Arc;

fn main() {
    let mutex = Arc::new(ReentrantMutex::new(()));
    let mut guard = mutex.lock();

    let mutex2 = Arc::clone(&mutex);

    std::thread::spawn(move || {
        let _guard = mutex2.lock();
    });

    // give the thread some time to try and lock
    std::thread::sleep(std::time::Duration::from_secs(2));

    ReentrantMutexGuard::bump(&mut guard);
}

Which results in the following traceback:

thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`', /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lock_api-0.4.10/src/remutex.rs:102:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:67:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/panicking.rs:228:5
   4: lock_api::remutex::RawReentrantMutex<R,G>::lock_internal
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/lock_api-0.4.10/src/remutex.rs:102:13
   5: lock_api::remutex::RawReentrantMutex<R,G>::lock
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/lock_api-0.4.10/src/remutex.rs:111:9
   6: lock_api::remutex::ReentrantMutex<R,G,T>::lock
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/lock_api-0.4.10/src/remutex.rs:313:9
   7: playground::main::{{closure}}
             at ./src/main.rs:12:22

I've tracked this down to RawReentrantMutex::bump(). It seems like it needs to set the lock_count to 0 before the inner bump() call, and then back to 1 after it. https://github.com/Amanieu/parking_lot/blob/04d6cfe2cac1d1db7385623865e9e6ed452ebf50/lock_api/src/remutex.rs#L182-L190

I'll send in a PR with a fix and a test