Mir

Code review comment for lp://qastaging/~alan-griffiths/mir/keep-helgrind-happy

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Helgrind uses the Eracer algorithm, which is pretty simple and even obvious if you think about it for a bit;
http://www.cs.duke.edu/courses/cps210/spring06/papers/eraser.pdf
As such it's very easy to trust the correctness of Helgrind, providing it has implemented hooks for all the lock/unlock functions you may depend on.

I'm not sure, but it appears the problem is that mutex is not guaranteed to be 1:1 to the switching_bundle which affects cv.

The basic rule for condition variables is that if you're not protecting any data/objects in the associated mutex then you're using the condition variable in a racy and unsafe way. This is why both C++ and pthreads always require a condition have an associated lock. Because failure to use that lock properly will make you race. I forget the exact reasoning, but it appears the solution would be to tie mutex to the switching bundle more closely. Even lock it before entering the function.

Using a static might be a safe solution, not sure, but it's definitely inefficient forcing all threads to share the same mutex. So not ideal. When/if I can commit more thought to the problem, I'm sure it would be possible to solve more correctly without "static".

review: Abstain

« Back to merge proposal