sync: document RWMutex.RLock shouldn't be used recursively

[dvyukov]

Detected by experimental deadlock detector in cl/77450043.

==================
WARNING: DEADLOCK
Goroutine 999 lock mutex 35 while holding mutex 35:
  sync.(*RWMutex).RLock()
      src/pkg/sync/rwmutex.go:42 +0xa2
  sync.(*rlocker).Lock()
      src/pkg/sync/rwmutex.go:133 +0x34
  sync_test.func·027()
      src/pkg/sync/rwmutex_test.go:132 +0xb6

Mutex 35 was previously locked here:
  sync.(*RWMutex).RLock()
      src/pkg/sync/rwmutex.go:42 +0xa2
  sync.(*rlocker).Lock()
      src/pkg/sync/rwmutex.go:133 +0x34
  sync_test.func·027()
      src/pkg/sync/rwmutex_test.go:133 +0xda
==================

==================
WARNING: DEADLOCK
Goroutine 999 lock mutex 16 while holding mutex 16:
  sync.(*RWMutex).RLock()
      src/pkg/sync/rwmutex.go:42 +0xa2
  sync_test.func·028()
      src/pkg/sync/rwmutex_test.go:166 +0x88
  testing.func·002()
      src/pkg/testing/benchmark.go:416 +0x17d

Mutex 16 was previously locked here:
  sync.(*RWMutex).RLock()
      src/pkg/sync/rwmutex.go:42 +0xa2
  sync_test.func·028()
      src/pkg/sync/rwmutex_test.go:167 +0x96
  testing.func·002()
      src/pkg/testing/benchmark.go:416 +0x17d
==================

Tests lock RWMutex recursively. RWMutex is not recursive.

[gopherbot]

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.

[dvyukov]

Comment 2:

By what design?
At least currently implementation does not support recursive read locking, and such code
indeed deadlocks.

[gopherbot]

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

[dvyukov]

Comment 4:

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

[bradfitz]

Comment 5:

How do you know who the reader is?
Goroutine? You can RLock in one goroutine and release in another, right? If that's
valid, I see no way for you to prohibit multiple RLocks by the "same" reader, however
that's (not) defined.

[gopherbot]

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.

[dvyukov]

Comment 7:

answered on golang-dev@ mailing list

[rsc]

Comment 8:

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.

[dvyukov]

Comment 9:

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.

[rsc]

Comment 10:

I don't think the test is pointless as written. It is testing that two RLocks can
coexist.