oklog/ulid

panic: runtime error: slice bounds out of range

GeorgeMac opened this issue · 4 comments

GeorgeMac commented

I keep getting panics from the underlying bufio.Reader when generating a new ulid.
This happens quite frequently now in a personal project of mine (https://github.com/GeorgeMac/adagio/tree/gm/recovery).

I have reproduced this with both v1.3.1 and v2 versions.

Is this to do with a lack of entropy?

agent_two_1  | panic: runtime error: slice bounds out of range
agent_two_1  |
agent_two_1  | goroutine 42 [running]:
agent_two_1  | bufio.(*Reader).Read(0xc0000ac2a0, 0xc0000a8fb6, 0xa, 0xa, 0x0, 0x0, 0x100000000000000)
agent_two_1  | 	/usr/local/go/src/bufio/bufio.go:234 +0x3c7
agent_two_1  | io.ReadAtLeast(0xbda320, 0xc0000ac2a0, 0xc0000a8fb6, 0xa, 0xa, 0xa, 0x0, 0xc0001fd518, 0x40db18)
agent_two_1  | 	/usr/local/go/src/io/io.go:310 +0x88
agent_two_1  | io.ReadFull(...)
agent_two_1  | 	/usr/local/go/src/io/io.go:329
agent_two_1  | github.com/oklog/ulid.(*monotonic).MonotonicRead(0xc0000ae3c0, 0x16c8faa24f1, 0xc0000a8fb6, 0xa, 0xa, 0xc0001fd578, 0x4b1c66)
agent_two_1  | 	/workspace/vendor/github.com/oklog/ulid/ulid.go:518 +0x8b
agent_two_1  | github.com/oklog/ulid.New(0x16c8faa24f1, 0xbda7a0, 0xc0000ae3c0, 0xc0001fd5c8, 0xc0001fd5c8, 0x43d9ed, 0xc00018e178)
agent_two_1  | 	/workspace/vendor/github.com/oklog/ulid/ulid.go:94 +0x125
agent_two_1  | github.com/oklog/ulid.MustNew(...)
agent_two_1  | 	/workspace/vendor/github.com/oklog/ulid/ulid.go:105
agent_two_1  | github.com/georgemac/adagio/pkg/worker.NewPool.func1(0xa351a0)
agent_two_1  | 	/workspace/pkg/worker/worker.go:62 +0x138
agent_two_1  | github.com/georgemac/adagio/pkg/worker.(*Pool).handleEvent(0xc0001e02a0, 0xc0000aff80, 0x2, 0x0)
agent_two_1  | 	/workspace/pkg/worker/worker.go:108 +0xa0
agent_two_1  | github.com/georgemac/adagio/pkg/worker.(*Pool).Run.func1(0xc0001de050, 0xc0001e02a0, 0xbea8c0, 0xc0000ae7c0)
agent_two_1  | 	/workspace/pkg/worker/worker.go:88 +0x19b
agent_two_1  | created by github.com/georgemac/adagio/pkg/worker.(*Pool).Run
agent_two_1  | 	/workspace/pkg/worker/worker.go:80 +0x96

UPDATE:

Expected Behaviour

Generation of a ulid to not lead to a bufio.ReadAll operation to panic.

Actual Behaviour

Concurrent use of ulid.MustNew(...) leads to the panic shown above.

Steps to Reproduce

package main

import (
	"fmt"
	"math/rand"
	"sync"
	"time"

	"github.com/oklog/ulid"
)

var entropy = ulid.Monotonic(rand.New(rand.NewSource(time.Now().UnixNano())), 0)

func main() {
	var wg sync.WaitGroup
	for i := 0; i < 1000; i++ {
		wg.Add(1)
		go func() {
			defer wg.Done()
			fmt.Println(ulid.MustNew(ulid.Timestamp(time.Now().UTC()), entropy))
		}()
	}
	wg.Wait()
}

Run the above (I realize now that it is actually when it is called concurrently)

See panic:

01DJ7VTPXP103NN85XXWR023FM
01DJ7VTPXP103NN85XZ9TC6G3S
panic: runtime error: slice bounds out of range

goroutine 37 [running]:
bufio.(*Reader).Read(0xc0000a40c0, 0xc00001c0e6, 0xa, 0xa, 0x0, 0x0, 0x100000000000000)
	/usr/local/Cellar/go/1.12.6/libexec/src/bufio/bufio.go:234 +0x3c7
io.ReadAtLeast(0x10e6860, 0xc0000a40c0, 0xc00001c0e6, 0xa, 0xa, 0xa, 0xc000054400, 0xc0000bc6d0, 0x100b0a8)
	/usr/local/Cellar/go/1.12.6/libexec/src/io/io.go:310 +0x88
io.ReadFull(...)
	/usr/local/Cellar/go/1.12.6/libexec/src/io/io.go:329
github.com/oklog/ulid.(*monotonic).MonotonicRead(0xc0000b4000, 0x16c8fbd5bb6, 0xc00001c0e6, 0xa, 0xa, 0xc0000bc730, 0x1081946)
	/Users/georgemac/go/pkg/mod/github.com/oklog/ulid@v1.3.1/ulid.go:518 +0x8b
github.com/oklog/ulid.New(0x16c8fbd5bb6, 0x10e68a0, 0xc0000b4000, 0x0, 0x0, 0x0, 0x0)
	/Users/georgemac/go/pkg/mod/github.com/oklog/ulid@v1.3.1/ulid.go:94 +0x125
github.com/oklog/ulid.MustNew(...)
	/Users/georgemac/go/pkg/mod/github.com/oklog/ulid@v1.3.1/ulid.go:105
main.main.func1(0xc0000b0010)
	/Users/georgemac/github/georgemac/ulidpanic/main.go:20 +0x153
created by main.main
	/Users/georgemac/github/georgemac/ulidpanic/main.go:18 +0x78
exit status 2

Additional Context

> uname -a                                                                      acceptance:(twodotoh)
Darwin Georges-MacBook-Pro.local 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64
> go version                                                                    acceptance:(twodotoh)
go version go1.12.6 darwin/amd64
> cat go.sum                                                                                                                                                                             
github.com/oklog/ulid v1.3.1 h1:EGfNDEx6MqHz8B3uNV6QAib1UR2Lm97sHi3ocA6ESJ4=
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
github.com/oklog/ulid/v2 v2.0.2 h1:r4fFzBm+bv0wNKNh5eXTwU7i85y5x+uwkxCUTNVQqLc=
github.com/oklog/ulid/v2 v2.0.2/go.mod h1:mtBL0Qe/0HAx6/a4Z30qxVIAL1eQDweXq5lxOEiwQ68=
github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
tsenart commented

Can you provide more details for us to be able to reproduce this bug?

Expected Behaviour

Actual Behaviour

Steps to Reproduce

  1. Step 1
  2. Step N

Additional Context

GeorgeMac commented

@tsenart I updated it 👍 managed to reproduce quite easilly with concurrent use of ulid.MustNew().

GeorgeMac commented

Do I need to protect concurrent access to my entropy is that it?

GeorgeMac commented

Just saw this:

Please note that rand.Rand from the math package is not safe for concurrent use. Instantiate one per long living go-routine or use a sync.Pool if you want to avoid the potential contention of a locked rand.Source as its been frequently observed in the package level functions.

My bad!