search

deckarep / golang-set

A simple, battle-tested and generic set type for the Go language. Trusted by Docker, 1Password, Ethereum and Hashicorp.
Other
4.15k stars 274 forks source link

New functions IsSuperset/IsProperSuperset were missing the appropriate read locks. #51

Closed deckarep closed 7 years ago

deckarep commented 7 years ago

For the ThreadSafe implementation of the Set, the new methods IsSuperset and IsProperSuperset needed the appropriate Read locks.

@spakin - if you'd like to review since it was your code initially.

spakin commented 7 years ago

Is this patch really correct? IsSuperset/IsProperSuperset immediately transfer control to IsSubset/IsProperSubset, which acquire read locks on both sets. The documentation for RWMutex states that double-locking isn't guaranteed to work:

If a goroutine holds a RWMutex for reading, it must not expect this or any other goroutine to be able to also take the read lock until the first read lock is released. In particular, this prohibits recursive read locking.

Am I missing something?

deckarep commented 7 years ago

@spakin - I think you're totally right actually...I forgot about that being the case here...

I'm going to Close this because you're statement makes total sense. Thanks!