3 ms·

Something I've always wondered - in the futures packages (https://github.com/Workiva/go-datastructures/blob/master/futures/futures.go https://github.com/Workiva

Something I've always wondered - in the futures packages (https://github.com/Workiva/go-datastructures/blob/master/futures/futures.go https://github.com/Workiva/go-datastructures/blob/master/fut...): The author has a variable `triggered` to check the state of whether or not the future is complete. Now to protect that variable from data races, the author uses a standard lock. Now since you need to protect a simple value what I would do in this situation is use an atomic variable w/ atomic.StoreInt32 (you could also probably not even have to use an atomic load in GetResult). Of course the best option would be to benchmark (and I'm assuming the author did, given that the structures are performant), but I'm wondering if my "gut" is right and under what situations others have seen that would necessitate a lock vs. an atomic instruction (high contention on readers, few writers)?

simonz05 11y ago
I think you are wrong in thinking he is protecting the `triggered` variable with the lock. He's using a Mutex to synchronize access on the entire struct. snippet from https://github.com/Workiva/go-datastructures/blob/master/futures/futures.go#L64 https://github.com/Workiva/go-datastructures/blob/master/fut... f.lock.Lock() f.triggered = true f.item = item f.err = err f.lock.Unlock() f.wg.Done()
- nemothekid 11y ago
  I'm confident the following is identical (if nothing else about the code changes). Notice how access to `f.item` and `f.err` aren't read protected in `GetResult`. func (f *Future) setItem(item interface{}, err error) { f.item = item f.err = err f.lock.Lock() f.triggered = true f.lock.Unlock() f.wg.Done() } Any reader of `f.item` or `f.err` (in `GetResult`) is essentially waiting for triggered to be true before reading the value of those respective fields. If after an atomic load or synchronized access of triggered returns `true`, both versions of `setItem` should guarantee that a subsequent read of `f.item` will return the "promised" value. In any case, because the reads (in `GetResult`) aren't protected as well, and the call to `setItem` which writes to `item` and `err` only happens on a single thread, the only thing the code perfectly protects now are reads of triggered.

simonz05 11y ago

I think you are wrong in thinking he is protecting the `triggered` variable with the lock. He's using a Mutex to synchronize access on the entire struct. snippet from https://github.com/Workiva/go-datastructures/blob/master/futures/futures.go#L64 https://github.com/Workiva/go-datastructures/blob/master/fut... f.lock.Lock() f.triggered = true f.item = item f.err = err f.lock.Unlock() f.wg.Done()

nemothekid 11y ago
I'm confident the following is identical (if nothing else about the code changes). Notice how access to `f.item` and `f.err` aren't read protected in `GetResult`. func (f *Future) setItem(item interface{}, err error) { f.item = item f.err = err f.lock.Lock() f.triggered = true f.lock.Unlock() f.wg.Done() } Any reader of `f.item` or `f.err` (in `GetResult`) is essentially waiting for triggered to be true before reading the value of those respective fields. If after an atomic load or synchronized access of triggered returns `true`, both versions of `setItem` should guarantee that a subsequent read of `f.item` will return the "promised" value. In any case, because the reads (in `GetResult`) aren't protected as well, and the call to `setItem` which writes to `item` and `err` only happens on a single thread, the only thing the code perfectly protects now are reads of triggered.