diff options
author | Daniel S Fava <danielsfava@gmail.com> | 2020-11-12 17:25:47 +0100 |
---|---|---|
committer | Ian Lance Taylor <iant@golang.org> | 2020-11-13 15:00:23 +0000 |
commit | 35455fff0ebb7dd1b8e698f245a823ef8c711ac9 (patch) | |
tree | f487de9d0a0fe87f557adeee844aa70f3e3734f0 /src/runtime/select.go | |
parent | 31f71506d7026595be76713af25197a8c0022ac8 (diff) | |
download | go-35455fff0ebb7dd1b8e698f245a823ef8c711ac9.tar.gz go-35455fff0ebb7dd1b8e698f245a823ef8c711ac9.zip |
runtime: swap the order of raceacquire() and racerelease()
In chansend() and chanrecv() of chan.go, the order of calls to
raceacquire() and racerelease() was swapped, which meant that the
code was not following the memory model "by the letter of the law."
Similar for bufrecv and bufsend in select.go
The memory model says:
- A send happens before the corresponding receive completes, and
- the kth receive on a channel with capacity C happens before the
k+C send on that channel completes.
The operative word here is "completes." For example, a sender obtains
happens-before information on completion of the send-operation, which
means, after the sender has deposited its message onto the channel.
Similarly for receives.
If the order of raceacquire() and racerelease() is incorrect, the race
detector may fail to report some race conditions.
The fix is minimal from the point of view of Go. The fix does, however,
rely on a new function added to TSan:
https://reviews.llvm.org/D76322
This commit only affects execution when race detection is enabled.
Added two tests into `runtime/race/output_test.go`:
- `chanmm` tests for the issue addressed by this patch
- `mutex` is a test for inverted semaphores, which must not be broken
by this (or any other) patch
Fixes #37355
Change-Id: I5e886879ead2bd456a4b7dd1d17253641b767f63
Reviewed-on: https://go-review.googlesource.com/c/go/+/220419
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Diffstat (limited to 'src/runtime/select.go')
-rw-r--r-- | src/runtime/select.go | 6 |
1 files changed, 2 insertions, 4 deletions
diff --git a/src/runtime/select.go b/src/runtime/select.go index 41e68a3746..f04b130b15 100644 --- a/src/runtime/select.go +++ b/src/runtime/select.go @@ -415,8 +415,7 @@ bufrecv: if cas.elem != nil { raceWriteObjectPC(c.elemtype, cas.elem, casePC(casi), chanrecvpc) } - raceacquire(chanbuf(c, c.recvx)) - racerelease(chanbuf(c, c.recvx)) + racereleaseacquire(chanbuf(c, c.recvx)) } if msanenabled && cas.elem != nil { msanwrite(cas.elem, c.elemtype.size) @@ -438,8 +437,7 @@ bufrecv: bufsend: // can send to buffer if raceenabled { - raceacquire(chanbuf(c, c.sendx)) - racerelease(chanbuf(c, c.sendx)) + racereleaseacquire(chanbuf(c, c.sendx)) raceReadObjectPC(c.elemtype, cas.elem, casePC(casi), chansendpc) } if msanenabled { |