Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible allocation issue on multiprocessor machines #17976

Closed
lummie opened this issue Nov 18, 2016 · 2 comments
Closed

Possible allocation issue on multiprocessor machines #17976

lummie opened this issue Nov 18, 2016 · 2 comments

Comments

@lummie
Copy link

lummie commented Nov 18, 2016

What version of Go are you using (go version)?

go version go1.7.3 linux/amd64

What operating system and processor architecture are you using (go env)?

linux/amd64
32 cpu - Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz

What did you do?

https://play.golang.org/p/jcpEGvgWFm

I have a Message structure that I am sending and receiving from a channel. the first go routine asks for Messages from a sync.pool, ensures the Values and Fields properties are empty and sends the message to a channel. When all the messages have been sent the channel is closed

The receiver uses range to take a message of the channel, alters the Fields and Values and then gives the message back to the pool.

Eventually a message from the pool is re-used and has Fields and Values set. These are then reset back to zero length map and array but sometimes this fails and the length of fields is > 0 causing the panic.

Note : this does not seem to exhibit the issue on the playground, however ti is fairly consistent on an SMP machine, certainly on my 16 core HT machine.

What did you expect to see?

I do not expect a panic to occur, the Fields and Values should be zero length

What did you see instead?

panic: Fields greater than zero
goroutine 4 [running]:
panic(0x459e40, 0xc42000a190)
/usr/local/go/src/runtime/panic.go:500 +0x1a1
main.main.func1(0xc420092060)
/home/I049472/godev/src/matt/epic/examples/test/test2.go:37 +0x1b4
created by main.main
/home/I049472/godev/src/matt/epic/examples/test/test2.go:42 +0x8a
exit status 2

Interestingly if I remove the modification of the Values array, which is obviously not even checked in the panic, then the issue does not arise.
https://play.golang.org/p/xwrJicCgYA

@dominikh
Copy link
Member

In for m := range ch, you have a single memory location m, that is the same for all iterations. When you messagePool.Put(&m), you store the address of that location in the pool. That means you can concurrently set m.Fields to an empty map and a non-empty map, writing to the same memory location. This is a race condition in your code.

@lummie
Copy link
Author

lummie commented Nov 18, 2016

thank you dominikh, sorry for my noobness.. If I change it to a chan *Message then I don't see the issue

@golang golang locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants