Fix RwLock::try_write #25153
Fix RwLock::try_write #25153
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
brson
added
the
beta-nominated
|
Nominating for beta by request of the author. |
There was a problem hiding this comment.
I believe that a poisoned rwlock will also return an Err even if it was successful in the try_read, so technically the lock could be locked if Err is returned. Perhaps the docs could be updated slightly to account for this?
There was a problem hiding this comment.
I think the pre-existing Failure section covers this, but it's outside the diff of this PR (https://github.com/jgallagher/rust/blob/rwlock-try-write/src/libstd/sync/rwlock.rs#L187-L190). Is what it states incorrect, or should this paragraph be reworded to include it somehow?
There was a problem hiding this comment.
Hm ok so upon re-reading my original though that this indicates Err means the lock wasn't grabbed isn't quite right, it just means that if it couldn't be grabbed Err is returned, so looks good to me!
|
Nice catch @jgallagher! Just one small nit from me on the doc updates and otherwise this is good to go. |
Previously, `try_write` actually only obtained shared read access (but would return a `RwLockWriteGuard` if that access was successful). Also updates the docs for `try_read` and `try_write`, which were leftover from when those methods returned `Option` instead of `Result`.
alexcrichton
added
the
beta-accepted
brson
removed
the
beta-nominated
6 participants
Previously,
try_writeactually only obtained shared read access (but would return aRwLockWriteGuardif that access was successful).Also updates the docs for
try_readandtry_write, which were leftover from when those methods returnedOptioninstead ofResult.