RLock 检查死锁存在问题: RWLock shouldn't check RLock order

[lycblank]

RLock与RUnlock如果存在循环引用时，会误报为deadlock。如: rlocka rlockb 协程A: rlocka rlockb

协程B: rlockb rlocka 此时会认为是死锁，但是它是读锁，不应该报deadlock

[sasha-s]

It seems the issue is about RLock triggering false positives. Using Google Translate on lycbank's original issue:

RLock and RUnlock will report deadlock if there is a circular reference. Such as:
Rlocka
Rlockb
A: rlocka
Rlockb

Association B: rlockb
Rlocka
This time will be considered deadlock, but it is read lock, should not be reported deadlock

[sasha-s]

The RLock can block, even if no write locks are taken.

The RWMutex is write-preferring. From RWMutex docs:

If a goroutine holds a RWMutex for reading and another goroutine might call Lock, no goroutine should expect to be able to acquire a read lock until the initial read lock is released. In particular, this prohibits recursive read locking. This is to ensure that the lock eventually becomes available; a blocked Lock call excludes new readers from acquiring the lock.

So, in the example with two two goroutines, doing RLock on two RWMutexes in different order, we could have a genuine deadlock if other goroutines try to Lock the mutexes (after the RLock is done).

I added a test (TestRWMutex) that illustrates this, with a single RWMutex.

I am inclined to keep the implementation as is: even when it is benign, doing RLocks in different order does not sound right, and it can lead to deadlocks because RWMutex is write-preferring.

[sasha-s]

Closing it for now.

I am inclined to keep the implementation as is: even when it is benign, doing RLocks in different order does not sound right, and it can lead to deadlocks because RWMutex is write-preferring.

[smileusd]

If the RWLock just use Rlock and RUnlock, it is correct way to use and not be deadlock, like this:

type MutexTestA struct {
    mutexA *deadlock.RWMutex
    mutexB *deadlock.RWMutex

}

func main() {
    mutexTest := MutexTestA{
        mutexA: &deadlock.RWMutex{},
        mutexB: &deadlock.RWMutex{},
    }
    deadlock.Opts.OnPotentialDeadlock = printINFO
    deadlock.Opts.DeadlockTimeout = time.Second * 20

    go mutexTest.testB()
    mutexTest.testA()
    fmt.Println("succeed")
    time.Sleep(time.Second * 10)
}

func printINFO() {
    fmt.Println("deadlock")
}

func (m *MutexTestA) testA() {
    m.mutexA.RLock()
    fmt.Println("get RlockA and release Rlock")
    time.Sleep(time.Second*2)
    m.mutexB.RLock()
    fmt.Println("get RlockB and release Rlock")
    defer m.mutexA.RUnlock()
    defer m.mutexB.RUnlock()
}

func (m *MutexTestA) testB() {
    m.mutexB.RLock()
    fmt.Println("get RlockB and release Rlock")
    time.Sleep(time.Second*2)
    m.mutexA.RLock()
    fmt.Println("get RlockA and release Rlock")
    defer m.mutexB.RUnlock()
    defer m.mutexA.RUnlock()
}

[sasha-s]

The following deadlocks:

    var a sync.RWMutex
    var b sync.RWMutex
    var wg sync.WaitGroup

    sleep := func() {
        time.Sleep(time.Duration((rand.Intn(20))) * time.Millisecond)
    }

    rlock12 := func(f, s sync.RWMutex) {
        defer wg.Done()
        sleep()
        f.RLock()
        sleep()
        s.RLock()
        sleep()
        s.RUnlock()
        sleep()
        f.RUnlock()
    }

    locka := func() {
        defer wg.Done()
        sleep()
        a.Lock()
        sleep()
        a.Unlock()
    }

    for i := 0; i < 100; i++ {
        wg.Add(3)
        go rlock12(a, b)
        go rlock12(b, a)
        go locka()
    }

    wg.Wait()

It might be difficult to define and detect benign behavior when read locks are grabbed out of order.

[smileusd]

Yes, it maybe need to separate "lock" into differrent operations for differrent lock types.