Ambiguity in RwLock documentation about multiple .read() calls on the same thread
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 119k
- Forks
- 16.1k
- PR merge metrics
- PR metrics pending
Description
Location (URL)
https://doc.rust-lang.org/std/sync/struct.RwLock.html#method.read
Summary
We're exploring backing RwLock with primitives from https://abseil.io/docs/cpp/guides/synchronization and encountered a surprise: RwLock is not documentedly reentrant for read lock holders, but there's an example that would fail if it wasn't. On the other hand, the documented "Potential deadlock example" is related. Then in the docs for fn read we see "This function might panic when called if the lock is already held by the current thread."
I think this means that it's not safe to call .read() on an RwLock multiple times in the same thread - but then there's this test in crossbeam-utils that fails on line 16 when the abseil deadlock detector is active, which is correct if the RwLock isn't reentrant.
Should we revise the documentation to clarify that RwLock isn't reentrant, since that's what's already implied? Or do we need to treat the example from crossbeam-utils as evidence that RwLock needs to be permissive in this regard even though it's a potential bug-in-waiting based on the deadlock example?
In favor of revising to clarify you shouldn't call .read() more than once per thread:
- leaves more flexibility for implementations delegating to OS-level constructs
- probably lets impls be more efficient
- lines up clearly with the potential deadlock example in the docs and makes adding deadlock detection more viable
In favor of clarifying to allow .read() more than once per thread:
- A little easier to use for users
- Hyrum's Law on the test in crossbeam-utils, which might be the tip of an iceberg
I'd be happy to send a doc update that makes the example use multiple threads rather than calling .read() (or add a comment describing how they should be in threads) and adds some warning text to .read() about only holding one reader lock per thread, but I wanted to clarify what direction we should go before doing so.
So: what's the consensus?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read the RwLock::read documentation and its linked examples, then compare them with the potential-deadlock example. Review the linked crossbeam-utils sharded_lock test as evidence of current behavior. The issue needs consensus on reentrancy before a documentation change can be considered done.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Documentation
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100