Skip to content

Commit ba048f7

Browse files
committed
sync: enable Pool when using race detector
Disabled by https://golang.org/cl/53020044 due to false positives. Reenable and model properly. Fixes #17306. Change-Id: I28405ddfcd17f58cf1427c300273212729154359 Reviewed-on: https://go-review.googlesource.com/31589 Run-TryBot: Russ Cox <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Dmitry Vyukov <[email protected]>
1 parent 154860f commit ba048f7

File tree

3 files changed

+101
-30
lines changed

3 files changed

+101
-30
lines changed
+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package race_test
6+
7+
import (
8+
"sync"
9+
"testing"
10+
"time"
11+
)
12+
13+
func TestRacePool(t *testing.T) {
14+
// Pool randomly drops the argument on the floor during Put.
15+
// Repeat so that at least one iteration gets reuse.
16+
for i := 0; i < 10; i++ {
17+
c := make(chan int)
18+
p := &sync.Pool{New: func() interface{} { return make([]byte, 10) }}
19+
x := p.Get().([]byte)
20+
x[0] = 1
21+
p.Put(x)
22+
go func() {
23+
y := p.Get().([]byte)
24+
y[0] = 2
25+
c <- 1
26+
}()
27+
x[0] = 3
28+
<-c
29+
}
30+
}
31+
32+
func TestNoRacePool(t *testing.T) {
33+
for i := 0; i < 10; i++ {
34+
p := &sync.Pool{New: func() interface{} { return make([]byte, 10) }}
35+
x := p.Get().([]byte)
36+
x[0] = 1
37+
p.Put(x)
38+
go func() {
39+
y := p.Get().([]byte)
40+
y[0] = 2
41+
p.Put(y)
42+
}()
43+
time.Sleep(100 * time.Millisecond)
44+
x = p.Get().([]byte)
45+
x[0] = 3
46+
}
47+
}

src/runtime/stubs.go

+3
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ var hashLoad = loadFactor
9696
// in asm_*.s
9797
func fastrand() uint32
9898

99+
//go:linkname sync_fastrand sync.fastrand
100+
func sync_fastrand() uint32 { return fastrand() }
101+
99102
// in asm_*.s
100103
//go:noescape
101104
func memequal(a, b unsafe.Pointer, size uintptr) bool

src/sync/pool.go

+51-30
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,49 @@ type poolLocal struct {
6161
pad [128]byte // Prevents false sharing.
6262
}
6363

64+
// from runtime
65+
func fastrand() uint32
66+
67+
var poolRaceHash [128]uint64
68+
69+
// poolRaceAddr returns an address to use as the synchronization point
70+
// for race detector logic. We don't use the actual pointer stored in x
71+
// directly, for fear of conflicting with other synchronization on that address.
72+
// Instead, we hash the pointer to get an index into poolRaceHash.
73+
// See discussion on golang.org/cl/31589.
74+
func poolRaceAddr(x interface{}) unsafe.Pointer {
75+
ptr := uintptr((*[2]unsafe.Pointer)(unsafe.Pointer(&x))[1])
76+
h := uint32((uint64(uint32(ptr)) * 0x85ebca6b) >> 16)
77+
return unsafe.Pointer(&poolRaceHash[h%uint32(len(poolRaceHash))])
78+
}
79+
6480
// Put adds x to the pool.
6581
func (p *Pool) Put(x interface{}) {
66-
if race.Enabled {
67-
// Under race detector the Pool degenerates into no-op.
68-
// It's conforming, simple and does not introduce excessive
69-
// happens-before edges between unrelated goroutines.
70-
return
71-
}
7282
if x == nil {
7383
return
7484
}
85+
if race.Enabled {
86+
if fastrand()%4 == 0 {
87+
// Randomly drop x on floor.
88+
return
89+
}
90+
race.ReleaseMerge(poolRaceAddr(x))
91+
race.Disable()
92+
}
7593
l := p.pin()
7694
if l.private == nil {
7795
l.private = x
7896
x = nil
7997
}
8098
runtime_procUnpin()
81-
if x == nil {
82-
return
99+
if x != nil {
100+
l.Lock()
101+
l.shared = append(l.shared, x)
102+
l.Unlock()
103+
}
104+
if race.Enabled {
105+
race.Enable()
83106
}
84-
l.Lock()
85-
l.shared = append(l.shared, x)
86-
l.Unlock()
87107
}
88108

89109
// Get selects an arbitrary item from the Pool, removes it from the
@@ -96,29 +116,34 @@ func (p *Pool) Put(x interface{}) {
96116
// the result of calling p.New.
97117
func (p *Pool) Get() interface{} {
98118
if race.Enabled {
99-
if p.New != nil {
100-
return p.New()
101-
}
102-
return nil
119+
race.Disable()
103120
}
104121
l := p.pin()
105122
x := l.private
106123
l.private = nil
107124
runtime_procUnpin()
108-
if x != nil {
109-
return x
125+
if x == nil {
126+
l.Lock()
127+
last := len(l.shared) - 1
128+
if last >= 0 {
129+
x = l.shared[last]
130+
l.shared = l.shared[:last]
131+
}
132+
l.Unlock()
133+
if x == nil {
134+
x = p.getSlow()
135+
}
110136
}
111-
l.Lock()
112-
last := len(l.shared) - 1
113-
if last >= 0 {
114-
x = l.shared[last]
115-
l.shared = l.shared[:last]
137+
if race.Enabled {
138+
race.Enable()
139+
if x != nil {
140+
race.Acquire(poolRaceAddr(x))
141+
}
116142
}
117-
l.Unlock()
118-
if x != nil {
119-
return x
143+
if x == nil && p.New != nil {
144+
x = p.New()
120145
}
121-
return p.getSlow()
146+
return x
122147
}
123148

124149
func (p *Pool) getSlow() (x interface{}) {
@@ -140,10 +165,6 @@ func (p *Pool) getSlow() (x interface{}) {
140165
}
141166
l.Unlock()
142167
}
143-
144-
if x == nil && p.New != nil {
145-
x = p.New()
146-
}
147168
return x
148169
}
149170

0 commit comments

Comments
 (0)