mirror of
https://github.com/golang/go
synced 2024-11-18 14:04:45 -07:00
sync: throw, not panic, for unlock of unlocked mutex
The panic leaves the lock in an unusable state. Trying to panic with a usable state makes the lock significantly less efficient and scalable (see early CL patch sets and discussion). Instead, use runtime.throw, which will crash the program directly. In general throw is reserved for when the runtime detects truly serious, unrecoverable problems. This problem is certainly serious, and, without a significant performance hit, is unrecoverable. Fixes #13879. Change-Id: I41920d9e2317270c6f909957d195bd8b68177f8d Reviewed-on: https://go-review.googlesource.com/31359 Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
parent
d2315fdc11
commit
40d81cf061
@ -576,6 +576,11 @@ func dopanic(unused int) {
|
|||||||
*(*int)(nil) = 0
|
*(*int)(nil) = 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
//go:linkname sync_throw sync.throw
|
||||||
|
func sync_throw(s string) {
|
||||||
|
throw(s)
|
||||||
|
}
|
||||||
|
|
||||||
//go:nosplit
|
//go:nosplit
|
||||||
func throw(s string) {
|
func throw(s string) {
|
||||||
print("fatal error: ", s, "\n")
|
print("fatal error: ", s, "\n")
|
||||||
|
@ -16,6 +16,8 @@ import (
|
|||||||
"unsafe"
|
"unsafe"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
func throw(string) // provided by runtime
|
||||||
|
|
||||||
// A Mutex is a mutual exclusion lock.
|
// A Mutex is a mutual exclusion lock.
|
||||||
// Mutexes can be created as part of other structures;
|
// Mutexes can be created as part of other structures;
|
||||||
// the zero value for a Mutex is an unlocked mutex.
|
// the zero value for a Mutex is an unlocked mutex.
|
||||||
@ -74,7 +76,7 @@ func (m *Mutex) Lock() {
|
|||||||
// The goroutine has been woken from sleep,
|
// The goroutine has been woken from sleep,
|
||||||
// so we need to reset the flag in either case.
|
// so we need to reset the flag in either case.
|
||||||
if new&mutexWoken == 0 {
|
if new&mutexWoken == 0 {
|
||||||
panic("sync: inconsistent mutex state")
|
throw("sync: inconsistent mutex state")
|
||||||
}
|
}
|
||||||
new &^= mutexWoken
|
new &^= mutexWoken
|
||||||
}
|
}
|
||||||
@ -108,7 +110,7 @@ func (m *Mutex) Unlock() {
|
|||||||
// Fast path: drop lock bit.
|
// Fast path: drop lock bit.
|
||||||
new := atomic.AddInt32(&m.state, -mutexLocked)
|
new := atomic.AddInt32(&m.state, -mutexLocked)
|
||||||
if (new+mutexLocked)&mutexLocked == 0 {
|
if (new+mutexLocked)&mutexLocked == 0 {
|
||||||
panic("sync: unlock of unlocked mutex")
|
throw("sync: unlock of unlocked mutex")
|
||||||
}
|
}
|
||||||
|
|
||||||
old := new
|
old := new
|
||||||
|
@ -7,7 +7,12 @@
|
|||||||
package sync_test
|
package sync_test
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
|
"internal/testenv"
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
"strings"
|
||||||
. "sync"
|
. "sync"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
@ -71,17 +76,98 @@ func TestMutex(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestMutexPanic(t *testing.T) {
|
var misuseTests = []struct {
|
||||||
defer func() {
|
name string
|
||||||
if recover() == nil {
|
f func()
|
||||||
t.Fatalf("unlock of unlocked mutex did not panic")
|
}{
|
||||||
}
|
{
|
||||||
}()
|
"Mutex.Unlock",
|
||||||
|
func() {
|
||||||
|
var mu Mutex
|
||||||
|
mu.Unlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"Mutex.Unlock2",
|
||||||
|
func() {
|
||||||
var mu Mutex
|
var mu Mutex
|
||||||
mu.Lock()
|
mu.Lock()
|
||||||
mu.Unlock()
|
mu.Unlock()
|
||||||
mu.Unlock()
|
mu.Unlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.Unlock",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.Unlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.Unlock2",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.RLock()
|
||||||
|
mu.Unlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.Unlock3",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.Lock()
|
||||||
|
mu.Unlock()
|
||||||
|
mu.Unlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.RUnlock",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.RUnlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.RUnlock2",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.Lock()
|
||||||
|
mu.RUnlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"RWMutex.RUnlock3",
|
||||||
|
func() {
|
||||||
|
var mu RWMutex
|
||||||
|
mu.RLock()
|
||||||
|
mu.RUnlock()
|
||||||
|
mu.RUnlock()
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
func init() {
|
||||||
|
if len(os.Args) == 3 && os.Args[1] == "TESTMISUSE" {
|
||||||
|
for _, test := range misuseTests {
|
||||||
|
if test.name == os.Args[2] {
|
||||||
|
test.f()
|
||||||
|
fmt.Printf("test completed\n")
|
||||||
|
os.Exit(0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
fmt.Printf("unknown test\n")
|
||||||
|
os.Exit(0)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMutexMisuse(t *testing.T) {
|
||||||
|
testenv.MustHaveExec(t)
|
||||||
|
for _, test := range misuseTests {
|
||||||
|
out, err := exec.Command(os.Args[0], "TESTMISUSE", test.name).CombinedOutput()
|
||||||
|
if err == nil || !strings.Contains(string(out), "unlocked") {
|
||||||
|
t.Errorf("%s: did not find failure with message about unlocked lock: %s\n%s\n", test.name, err, out)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func BenchmarkMutexUncontended(b *testing.B) {
|
func BenchmarkMutexUncontended(b *testing.B) {
|
||||||
|
@ -61,7 +61,7 @@ func (rw *RWMutex) RUnlock() {
|
|||||||
if r := atomic.AddInt32(&rw.readerCount, -1); r < 0 {
|
if r := atomic.AddInt32(&rw.readerCount, -1); r < 0 {
|
||||||
if r+1 == 0 || r+1 == -rwmutexMaxReaders {
|
if r+1 == 0 || r+1 == -rwmutexMaxReaders {
|
||||||
race.Enable()
|
race.Enable()
|
||||||
panic("sync: RUnlock of unlocked RWMutex")
|
throw("sync: RUnlock of unlocked RWMutex")
|
||||||
}
|
}
|
||||||
// A writer is pending.
|
// A writer is pending.
|
||||||
if atomic.AddInt32(&rw.readerWait, -1) == 0 {
|
if atomic.AddInt32(&rw.readerWait, -1) == 0 {
|
||||||
@ -115,7 +115,7 @@ func (rw *RWMutex) Unlock() {
|
|||||||
r := atomic.AddInt32(&rw.readerCount, rwmutexMaxReaders)
|
r := atomic.AddInt32(&rw.readerCount, rwmutexMaxReaders)
|
||||||
if r >= rwmutexMaxReaders {
|
if r >= rwmutexMaxReaders {
|
||||||
race.Enable()
|
race.Enable()
|
||||||
panic("sync: Unlock of unlocked RWMutex")
|
throw("sync: Unlock of unlocked RWMutex")
|
||||||
}
|
}
|
||||||
// Unblock blocked readers, if any.
|
// Unblock blocked readers, if any.
|
||||||
for i := 0; i < int(r); i++ {
|
for i := 0; i < int(r); i++ {
|
||||||
|
@ -155,48 +155,6 @@ func TestRLocker(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestUnlockPanic(t *testing.T) {
|
|
||||||
defer func() {
|
|
||||||
if recover() == nil {
|
|
||||||
t.Fatalf("unlock of unlocked RWMutex did not panic")
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
var mu RWMutex
|
|
||||||
mu.Unlock()
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestUnlockPanic2(t *testing.T) {
|
|
||||||
defer func() {
|
|
||||||
if recover() == nil {
|
|
||||||
t.Fatalf("unlock of unlocked RWMutex did not panic")
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
var mu RWMutex
|
|
||||||
mu.RLock()
|
|
||||||
mu.Unlock()
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestRUnlockPanic(t *testing.T) {
|
|
||||||
defer func() {
|
|
||||||
if recover() == nil {
|
|
||||||
t.Fatalf("read unlock of unlocked RWMutex did not panic")
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
var mu RWMutex
|
|
||||||
mu.RUnlock()
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestRUnlockPanic2(t *testing.T) {
|
|
||||||
defer func() {
|
|
||||||
if recover() == nil {
|
|
||||||
t.Fatalf("read unlock of unlocked RWMutex did not panic")
|
|
||||||
}
|
|
||||||
}()
|
|
||||||
var mu RWMutex
|
|
||||||
mu.Lock()
|
|
||||||
mu.RUnlock()
|
|
||||||
}
|
|
||||||
|
|
||||||
func BenchmarkRWMutexUncontended(b *testing.B) {
|
func BenchmarkRWMutexUncontended(b *testing.B) {
|
||||||
type PaddedRWMutex struct {
|
type PaddedRWMutex struct {
|
||||||
RWMutex
|
RWMutex
|
||||||
|
Loading…
Reference in New Issue
Block a user