From: Aliaksandr Valialkin Date: Thu, 14 Apr 2016 21:33:28 +0000 (+0300) Subject: cmd/vet: check sync.* types' copying X-Git-Tag: go1.7beta1~297 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=c81a3532fea42df33dea54497dfaa96873c2d976;p=gostls13.git cmd/vet: check sync.* types' copying Embed noLock struct into the following types, so `go vet -copylocks` catches their copying additionally to types containing sync.Mutex: - sync.Cond - sync.WaitGroup - sync.Pool - atomic.Value Fixes #14582 Change-Id: Icb543ef5ad10524ad239a15eec8a9b334b0e0660 Reviewed-on: https://go-review.googlesource.com/22015 Reviewed-by: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Gobot Gobot --- diff --git a/src/cmd/vet/testdata/copylock.go b/src/cmd/vet/testdata/copylock.go index cf56802cdb..d49f468627 100644 --- a/src/cmd/vet/testdata/copylock.go +++ b/src/cmd/vet/testdata/copylock.go @@ -1,6 +1,9 @@ package testdata -import "sync" +import ( + "sync" + "sync/atomic" +) func OkFunc() { var x *sync.Mutex @@ -66,3 +69,72 @@ func BadFunc() { new := func(interface{}) {} new(t) // ERROR "function call copies lock value: testdata.Tlock contains sync.Once contains sync.Mutex" } + +// SyncTypesCheck checks copying of sync.* types except sync.Mutex +func SyncTypesCheck() { + // sync.RWMutex copying + var rwmuX sync.RWMutex + var rwmuXX = sync.RWMutex{} + rwmuX1 := new(sync.RWMutex) + rwmuY := rwmuX // ERROR "assignment copies lock value to rwmuY: sync.RWMutex" + rwmuY = rwmuX // ERROR "assignment copies lock value to rwmuY: sync.RWMutex" + var rwmuYY = rwmuX // ERROR "variable declaration copies lock value to rwmuYY: sync.RWMutex" + rwmuP := &rwmuX + rwmuZ := &sync.RWMutex{} + + // sync.Cond copying + var condX sync.Cond + var condXX = sync.Cond{} + condX1 := new(sync.Cond) + condY := condX // ERROR "assignment copies lock value to condY: sync.Cond contains sync.noCopy" + condY = condX // ERROR "assignment copies lock value to condY: sync.Cond contains sync.noCopy" + var condYY = condX // ERROR "variable declaration copies lock value to condYY: sync.Cond contains sync.noCopy" + condP := &condX + condZ := &sync.Cond{ + L: &sync.Mutex{}, + } + condZ = sync.NewCond(&sync.Mutex{}) + + // sync.WaitGroup copying + var wgX sync.WaitGroup + var wgXX = sync.WaitGroup{} + wgX1 := new(sync.WaitGroup) + wgY := wgX // ERROR "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy" + wgY = wgX // ERROR "assignment copies lock value to wgY: sync.WaitGroup contains sync.noCopy" + var wgYY = wgX // ERROR "variable declaration copies lock value to wgYY: sync.WaitGroup contains sync.noCopy" + wgP := &wgX + wgZ := &sync.WaitGroup{} + + // sync.Pool copying + var poolX sync.Pool + var poolXX = sync.Pool{} + poolX1 := new(sync.Pool) + poolY := poolX // ERROR "assignment copies lock value to poolY: sync.Pool contains sync.noCopy" + poolY = poolX // ERROR "assignment copies lock value to poolY: sync.Pool contains sync.noCopy" + var poolYY = poolX // ERROR "variable declaration copies lock value to poolYY: sync.Pool contains sync.noCopy" + poolP := &poolX + poolZ := &sync.Pool{} + + // sync.Once copying + var onceX sync.Once + var onceXX = sync.Once{} + onceX1 := new(sync.Once) + onceY := onceX // ERROR "assignment copies lock value to onceY: sync.Once contains sync.Mutex" + onceY = onceX // ERROR "assignment copies lock value to onceY: sync.Once contains sync.Mutex" + var onceYY = onceX // ERROR "variable declaration copies lock value to onceYY: sync.Once contains sync.Mutex" + onceP := &onceX + onceZ := &sync.Once{} +} + +// AtomicTypesCheck checks copying of sync/atomic types +func AtomicTypesCheck() { + // atomic.Value copying + var vX atomic.Value + var vXX = atomic.Value{} + vX1 := new(atomic.Value) + vY := vX // ERROR "assignment copies lock value to vY: sync/atomic.Value contains sync/atomic.noCopy" + vY = vX // ERROR "assignment copies lock value to vY: sync/atomic.Value contains sync/atomic.noCopy" + var vYY = vX // ERROR "variable declaration copies lock value to vYY: sync/atomic.Value contains sync/atomic.noCopy" + vP := &vX + vZ := &atomic.Value{} +} diff --git a/src/sync/atomic/value.go b/src/sync/atomic/value.go index ab3aa11285..30abf72634 100644 --- a/src/sync/atomic/value.go +++ b/src/sync/atomic/value.go @@ -12,7 +12,11 @@ import ( // Values can be created as part of other data structures. // The zero value for a Value returns nil from Load. // Once Store has been called, a Value must not be copied. +// +// A Value must not be copied after first use. type Value struct { + noCopy noCopy + v interface{} } @@ -83,3 +87,13 @@ func (v *Value) Store(x interface{}) { // Disable/enable preemption, implemented in runtime. func runtime_procPin() func runtime_procUnpin() + +// noCopy may be embedded into structs which must not be copied +// after the first use. +// +// See https://github.com/golang/go/issues/8005#issuecomment-190753527 +// for details. +type noCopy struct{} + +// Lock is a no-op used by -copylocks checker from `go vet`. +func (*noCopy) Lock() {} diff --git a/src/sync/cond.go b/src/sync/cond.go index f711c39da2..c070d9d84e 100644 --- a/src/sync/cond.go +++ b/src/sync/cond.go @@ -20,6 +20,8 @@ import ( // A Cond can be created as part of other structures. // A Cond must not be copied after first use. type Cond struct { + noCopy noCopy + // L is held while observing or changing the condition L Locker @@ -84,3 +86,13 @@ func (c *copyChecker) check() { panic("sync.Cond is copied") } } + +// noCopy may be embedded into structs which must not be copied +// after the first use. +// +// See https://github.com/golang/go/issues/8005#issuecomment-190753527 +// for details. +type noCopy struct{} + +// Lock is a no-op used by -copylocks checker from `go vet`. +func (*noCopy) Lock() {} diff --git a/src/sync/mutex.go b/src/sync/mutex.go index 78b115cf5a..90892793f0 100644 --- a/src/sync/mutex.go +++ b/src/sync/mutex.go @@ -19,6 +19,8 @@ import ( // A Mutex is a mutual exclusion lock. // Mutexes can be created as part of other structures; // the zero value for a Mutex is an unlocked mutex. +// +// A Mutex must not be copied after first use. type Mutex struct { state int32 sema uint32 diff --git a/src/sync/pool.go b/src/sync/pool.go index 2acf505f3c..bf29d88c5c 100644 --- a/src/sync/pool.go +++ b/src/sync/pool.go @@ -40,7 +40,10 @@ import ( // that scenario. It is more efficient to have such objects implement their own // free list. // +// A Pool must not be copied after first use. type Pool struct { + noCopy noCopy + local unsafe.Pointer // local fixed-size per-P pool, actual type is [P]poolLocal localSize uintptr // size of the local array diff --git a/src/sync/rwmutex.go b/src/sync/rwmutex.go index 9fc6e3bd2c..455d412330 100644 --- a/src/sync/rwmutex.go +++ b/src/sync/rwmutex.go @@ -16,6 +16,8 @@ import ( // RWMutexes can be created as part of other // structures; the zero value for a RWMutex is // an unlocked mutex. +// +// An RWMutex must not be copied after first use. type RWMutex struct { w Mutex // held if there are pending writers writerSem uint32 // semaphore for writers to wait for completing readers diff --git a/src/sync/waitgroup.go b/src/sync/waitgroup.go index 029e6077cd..b386e1fec2 100644 --- a/src/sync/waitgroup.go +++ b/src/sync/waitgroup.go @@ -15,7 +15,11 @@ import ( // goroutines to wait for. Then each of the goroutines // runs and calls Done when finished. At the same time, // Wait can be used to block until all goroutines have finished. +// +// A WaitGroup must not be copied after first use. type WaitGroup struct { + noCopy noCopy + // 64-bit value: high 32 bits are counter, low 32 bits are waiter count. // 64-bit atomic operations require 64-bit alignment, but 32-bit // compilers do not ensure it. So we allocate 12 bytes and then use