Repository metrics
- Stars
- (133,883 stars)
- PR merge metrics
- (No merged PRs in 30d)
Description
Thinking about https://github.com/golang/go/issues/20678 (and #19624) made me wonder what other unchecked overflows are lurking in the standard library. I've found another one, but I don't have real-world code that triggers it. (I'm filing this issue just for reference.)
(*sync.WaitGroup).Add accepts an int parameter, so it is reasonable for users to expect that they cannot make Add calls summing to more than the maximum possible int. However, in practice WaitGroup only supports Add calls that sum to within math.MaxInt32.
Add currently detects (and panics on) overflows that happen to land in the negative half of the int32 space, but fails to detect other overflows entirely.
Add should be fixed to reliably panic on all overflows. It should ideally support the full positive int range, but if that isn't feasible the supported range should be made clear in the documentation.
$ go version
go version devel +0a0e45d5c6 Thu May 25 17:57:21 2017 -0400 linux/amd64
wgover/wgover.go:
package main
import (
"fmt"
"math"
"sync"
"sync/atomic"
)
func main() {
var wg sync.WaitGroup
var n int64
const maxInt = int(^uint(0) >> 1)
iters := 0x40000000
if int64(maxInt) > math.MaxInt32 {
iters <<= 2
}
fmt.Printf("looping for %d iterations\n", iters)
wg.Add(iters)
atomic.AddInt64(&n, int64(iters))
go func() {
for {
wg.Done()
if atomic.AddInt64(&n, -1) == 0 {
break
}
}
}()
wg.Wait()
fmt.Printf("done with %d iterations remaining\n", atomic.LoadInt64(&n))
}
expected (with very long running time):
$ go run wgover/wgover.go
looping for 4294967296 iterations
done with 0 iterations remaining
actual:
$ go run wgover/wgover.go
looping for 4294967296 iterations
done with 4294967296 iterations remaining