4 ms·
I've only looked at the queue implementation, but both push and pop contain obvious race conditions; I would highly suggest adding tests that actually use the d
by whiteboardma 3y ago
I've only looked at the queue implementation, but both push and pop contain obvious race conditions; I would highly suggest adding tests that actually use the data structures from multiple threads.
- dnedic 3y agoCould you elaborate on the alleged race conditions? Any advice on reliably testing the race conditions? The problem with adding those is the fact that they will give lots of false negatives and if you rely on them you have a problem.
- whiteboardma 3y agoLooking at the Push operation defined in queue_impl.hpp, if multiple threads perform concurrent pushes, they might end up writing their element to the same slot in _data since the current position _w is not incremented atomically
- dnedic 3y agoThis is a multi producer scenario, the README clearly states that these data structures are only single producer single consumer multi thread/interrupt safe. I will also add disclaimers to the javadocs comments on methods just to reduce confusion.
- whiteboardma 3y agoWops, my bad
- ape4 3y agoJust add a lock ;)
- gjadi 3y agoYou could use TLA+ to model the data structure operations and check the invariant. Checking the invariant with assert is also useful in my limited experience with concurrency. https://lamport.azurewebsites.net/tla/tla.html https://lamport.azurewebsites.net/tla/tla.html
- dnedic 3y agoThanks a lot, will check this out!