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 <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

diff --git a/src/runtime/race/testdata/pool_test.go b/src/runtime/race/testdata/pool_test.go
new file mode 100644 (file)
index 0000000..161f4b7
--- /dev/null
+++ b/src/runtime/race/testdata/pool_test.go
@@ -0,0 +1,47 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package race_test
+
+import (
+       "sync"
+       "testing"
+       "time"
+)
+
+func TestRacePool(t *testing.T) {
+       // Pool randomly drops the argument on the floor during Put.
+       // Repeat so that at least one iteration gets reuse.
+       for i := 0; i < 10; i++ {
+               c := make(chan int)
+               p := &sync.Pool{New: func() interface{} { return make([]byte, 10) }}
+               x := p.Get().([]byte)
+               x[0] = 1
+               p.Put(x)
+               go func() {
+                       y := p.Get().([]byte)
+                       y[0] = 2
+                       c <- 1
+               }()
+               x[0] = 3
+               <-c
+       }
+}
+
+func TestNoRacePool(t *testing.T) {
+       for i := 0; i < 10; i++ {
+               p := &sync.Pool{New: func() interface{} { return make([]byte, 10) }}
+               x := p.Get().([]byte)
+               x[0] = 1
+               p.Put(x)
+               go func() {
+                       y := p.Get().([]byte)
+                       y[0] = 2
+                       p.Put(y)
+               }()
+               time.Sleep(100 * time.Millisecond)
+               x = p.Get().([]byte)
+               x[0] = 3
+       }
+}

diff --git a/src/runtime/stubs.go b/src/runtime/stubs.go
index 7384c7810f33ee1722210bf8299f550b49dc0815..107f2604b16124309a4c8571406d1e2d54542fa3 100644 (file)
--- a/src/runtime/stubs.go
+++ b/src/runtime/stubs.go
@@ -96,6 +96,9 @@ var hashLoad = loadFactor
 // in asm_*.s
 func fastrand() uint32
 
+//go:linkname sync_fastrand sync.fastrand
+func sync_fastrand() uint32 { return fastrand() }
+
 // in asm_*.s
 //go:noescape
 func memequal(a, b unsafe.Pointer, size uintptr) bool

diff --git a/src/sync/pool.go b/src/sync/pool.go
index bf29d88c5cb6ccfb926d8735c934c482c54d3e2c..0acdbde096f07dc1b89b244dbd3e6b9ff4389e15 100644 (file)
--- a/src/sync/pool.go
+++ b/src/sync/pool.go
@@ -61,29 +61,49 @@ type poolLocal struct {
        pad     [128]byte     // Prevents false sharing.
 }
 
+// from runtime
+func fastrand() uint32
+
+var poolRaceHash [128]uint64
+
+// poolRaceAddr returns an address to use as the synchronization point
+// for race detector logic. We don't use the actual pointer stored in x
+// directly, for fear of conflicting with other synchronization on that address.
+// Instead, we hash the pointer to get an index into poolRaceHash.
+// See discussion on golang.org/cl/31589.
+func poolRaceAddr(x interface{}) unsafe.Pointer {
+       ptr := uintptr((*[2]unsafe.Pointer)(unsafe.Pointer(&x))[1])
+       h := uint32((uint64(uint32(ptr)) * 0x85ebca6b) >> 16)
+       return unsafe.Pointer(&poolRaceHash[h%uint32(len(poolRaceHash))])
+}
+
 // Put adds x to the pool.
 func (p *Pool) Put(x interface{}) {
-       if race.Enabled {
-               // Under race detector the Pool degenerates into no-op.
-               // It's conforming, simple and does not introduce excessive
-               // happens-before edges between unrelated goroutines.
-               return
-       }
        if x == nil {
                return
        }
+       if race.Enabled {
+               if fastrand()%4 == 0 {
+                       // Randomly drop x on floor.
+                       return
+               }
+               race.ReleaseMerge(poolRaceAddr(x))
+               race.Disable()
+       }
        l := p.pin()
        if l.private == nil {
                l.private = x
                x = nil
        }
        runtime_procUnpin()
-       if x == nil {
-               return
+       if x != nil {
+               l.Lock()
+               l.shared = append(l.shared, x)
+               l.Unlock()
+       }
+       if race.Enabled {
+               race.Enable()
        }
-       l.Lock()
-       l.shared = append(l.shared, x)
-       l.Unlock()
 }
 
 // Get selects an arbitrary item from the Pool, removes it from the
@@ -96,29 +116,34 @@ func (p *Pool) Put(x interface{}) {
 // the result of calling p.New.
 func (p *Pool) Get() interface{} {
        if race.Enabled {
-               if p.New != nil {
-                       return p.New()
-               }
-               return nil
+               race.Disable()
        }
        l := p.pin()
        x := l.private
        l.private = nil
        runtime_procUnpin()
-       if x != nil {
-               return x
+       if x == nil {
+               l.Lock()
+               last := len(l.shared) - 1
+               if last >= 0 {
+                       x = l.shared[last]
+                       l.shared = l.shared[:last]
+               }
+               l.Unlock()
+               if x == nil {
+                       x = p.getSlow()
+               }
        }
-       l.Lock()
-       last := len(l.shared) - 1
-       if last >= 0 {
-               x = l.shared[last]
-               l.shared = l.shared[:last]
+       if race.Enabled {
+               race.Enable()
+               if x != nil {
+                       race.Acquire(poolRaceAddr(x))
+               }
        }
-       l.Unlock()
-       if x != nil {
-               return x
+       if x == nil && p.New != nil {
+               x = p.New()
        }
-       return p.getSlow()
+       return x
 }
 
 func (p *Pool) getSlow() (x interface{}) {
@@ -140,10 +165,6 @@ func (p *Pool) getSlow() (x interface{}) {
                }
                l.Unlock()
        }
-
-       if x == nil && p.New != nil {
-               x = p.New()
-       }
        return x
 }