New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sync: document RWMutex.RLock shouldn't be used recursively #7576
Comments
Comment 1 by 2852914@wessie.info: This looks like a false positive to me. The test locks the `rlocker` multiple times, which is the read lock from `RWMutex.RLocker()`. By design the read lock from a RWMutex is allowed to be acquired multiple times. |
Comment 3 by 2852914@wessie.info: The documentation http://golang.org/pkg/sync/#RWMutex tells you that "The lock can be held by an arbitrary number of readers or a single writer". It's also easily demonstrated by a small example such as http://play.golang.org/p/TteARoVJeh |
it does not say that the mutex can be locked several times by a single reader this is a common thing for rw mutexes, you can see that SRWLOCK provides the same guarantee: "SRW locks cannot be acquired recursively" http://msdn.microsoft.com/en-us/library/windows/desktop/aa904937(v=vs.85).aspx |
Comment 6 by 2852914@wessie.info: So this can still count as a potential deadlock unless the documentation is updated to guarantee the current behavior. Which is currently implicitly given by the fact that RLock and RUnlock don't have to be paired in the same goroutine. |
I think the test is fine. I agree that RLock cannot be called recursively if someone else might be trying to Lock at the same time. But in these two instances the test has been written so that there are no concurrent calls to Lock. In that Lock-free context, duplicate RLocks are safe and I think should be expected to work. Status changed to WorkingAsIntended. |
We will need it if/when we deploy deadlock detector. Locking a mutex when nobody else locks it is pointless, so even if this scenario works, it is not testing anything useful. And also demonstrates a bad example to readers. Labels changed: added release-none. Owner changed to @dvyukov. Status changed to Accepted. |
@dvyukov, should we repurpose this into a bug to clarify some documentation on https://golang.org/pkg/sync/#RWMutex.RLock to say that it must not be used recursively by the same caller? |
The documentation already says this:
|
The text was updated successfully, but these errors were encountered: