ugorji / go

idiomatic codec and rpc lib for msgpack, cbor, json, etc. msgpack.org[Go]
MIT License
1.86k stars 295 forks source link

race in `basicHandle()` #292

Closed zeldovich closed 5 years ago

zeldovich commented 5 years ago

basicHandle() in codec/helper.go uses an atomic compare-and-swap to ensure initialization gets called only once, but does not wait for initialization to finish, leading to a race condition.

Suppose two goroutines use a shared var h MsgpackHandle and run code like this:

var b []byte
enc := codec.NewEncoderBytes(&b, &h)
enc.MustEncode(123)

Suppose further that the first goroutine to execute gets as far as basicHandle being called from NewEncoderBytes, and gets suspended right after its successful call to atomic.CompareAndSwapUint32. Then the second goroutine will bypass this initialization logic, but will not get a fully-initialized handle (e.g., x.be is not yet set correctly). If the first goroutine now wakes up, its writes in basicHandle will now race with reads by the second goroutine.

ugorji commented 5 years ago

I am not sure what you are talking about.

Can you reference some lines in code, and/or provide a reproducer?

Thanks.

ugorji commented 5 years ago

Ah - I see it now. Thanks.

ugorji commented 5 years ago

I was hoping to avoid using once.Do, but it has been streamlined to inline the fast path now, so I will use it. Fix coming soon.