[go] database/sql: avoid deadlock from reentrant RLock

0 views

Skip to first unread message

Gopher Robot (Gerrit)

unread,

4:28 PM (7 hours ago) 4:28 PM

to Damien Neil, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Neal Patel, golang-co...@googlegroups.com

Gopher Robot submitted the change with unreviewed changes

Unreviewed changes

3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/database/sql/closemu_test.go
Insertions: 34, Deletions: 0.

@@ -90,3 +90,37 @@
 
 	})
 }
+
+func TestClosingMutexPanics(t *testing.T) {
+	for _, test := range []struct {
+		name string
+		f    func()
+	}{{
+		name: "double RUnlock",
+		f: func() {
+			var m closingMutex
+			m.RLock()
+			m.RUnlock()
+			m.RUnlock()
+		},
+	}, {
+		name: "double Unlock",
+		f: func() {
+			var m closingMutex
+			m.Lock()
+			m.Unlock()
+			m.Unlock()
+		},
+	}} {
+		var got any
+		func() {
+			defer func() {
+				got = recover()
+			}()
+			test.f()
+		}()
+		if got == nil {
+			t.Errorf("no panic, want one")
+		}
+	}
+}
```
```
The name of the file: src/database/sql/closemu.go
Insertions: 2, Deletions: 2.

@@ -15,7 +15,7 @@
 type closingMutex struct {
 	// state is 2*readers+writerWaiting.
 	//   0 is unlocked
-	//   1 is impossible
+	//   1 is unlocked and a writer needs to wake
 	//   >0 is read-locked
 	//   <0 is write-locked
 	state atomic.Int64
@@ -48,7 +48,7 @@
 			panic("runlock of un-rlocked mutex")
 		}
 		if m.state.CompareAndSwap(x, x-2) {
-			if x == 3 {
+			if x-2 == 1 {
 				// We were the last reader, and a writer is waiting.
 				// The lock makes sure the writer sees the broadcast.
 				m.mu.Lock()
```

Change information

Commit message:

database/sql: avoid deadlock from reentrant RLock

RWMutex.RLock blocks until any pending Lock operations are satisfied.
This prohibits recursive read-locking.  Replace various RWMutexes
used to synchronize between reads and closes with a variant where
the reader side takes priority. Reads can starve out Close, but will
not deadlock.

Fixes #78304

Change-Id: Id36529bf86bed5dbf22f2af94283aeac6a6a6964

Reviewed-on: https://go-review.googlesource.com/c/go/+/758064

Auto-Submit: Damien Neil <dn...@google.com>

Reviewed-by: Neal Patel <neal...@google.com>

LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Files: