search

bertptrs / tracing-mutex

A Mutex wrapper tracking acquisition order
https://docs.rs/tracing-mutex/
Apache License 2.0
66 stars 7 forks source link

Re-entrant locking is not detected as a cycle #7

Closed quisar closed 2 years ago

quisar commented 2 years ago

Most locks in rust are not re-entrants according to their documentation.

The code in DiGraph explicitly allows a link for x to x.

Could this be changed so that this library can detect when the client is taking a lock it already has?

Thanks.

bertptrs commented 2 years ago

I think the self-cycle should be disallowed in general, as they are prone to cause RWR deadlocks for RwLocks (for example https://github.com/Amanieu/parking_lot/issues/212). These kind of cycles are already detected when the the length is >1, but cycles of length 1 can also be problematic.

Reentrant mutexes might still be useful but they're currently disallowed for cycles of length >1 and for the case where the cycle is of length 1, it should be more performant to pass the guard along in the first place.

In short, I think the "proper" solution for the time being is to disallow self-cycles altogether, rather than optionally (dis)allowing them on a case-by-case basis.

quisar commented 2 years ago

Done. I updated the CL for this.

Another solution would be to check the stack of current lock for recursive locks and not register anything if the lock is already there. But I agree that not allowing it at all is fine until someone needs more.