|
From: Philippe W. <phi...@sk...> - 2012-11-05 22:18:57
|
On Mon, 2012-11-05 at 18:59 +0100, David Faure wrote: > The testcase http://www.davidfaure.fr/2012/qmutex_trylock.cpp > (from https://bugs.kde.org/show_bug.cgi?id=243232) > shows that an optimization inside Qt leads to a helgrind warning about wrong > lock ordering, making the use of that feature impossible for detecting actual > problems elsewhere (i.e. I have to use --track-lockorders=no all the time). > > Technically if we ignore the distinction between lock and tryLock, helgrind is > right, we did lock the mutexes in reverse order. But because it's a tryLock, > it couldn't possibly deadlock. > > Should helgrind simply ignore all pthread_mutex_trylock calls, for the > lockorder detection, even if they succeed? I think so, actually (by definition > they couldn't deadlock, which is what track-lockorders is all about). A deadlock can appear if there is a mixture of trylock and lock on the same lock. So, trylock cannot just be ignored. E.g. Thread 1: trylock mutex1 lock mutex2 Thread 2: lock mutex2 lock mutex1 might deadlock. Even without mixture, isn't the below slightly bizarre/dangerous ? Thread 1: trylock mutex1 trylock mutex2 Thread 2: trylock mutex2 trylock mutex1 If the 2nd trylock fails, what is the "plan B" ? It seems that a task must unlock all locks and restart from "scratch" in the above case. I guess we might need an option such as: --trylock-logic={normal-lock|local-retry|full-retry} normal-lock = current behaviour local-retry means the task would re-trylock full-retry means that the plan B is to unlock all locks and retry everything. Or maybe we would need the same info but with an annotation of the lock operation and/or of the lock itself ? I am quite sure the simple logic "trylock can be ignored" is not ok for all cases. > PS: see my previous email about VALGRIND_HG_ENABLE_CHECKING not working. Is > this known? Should I report a bug? Always good to report a bug if you have found a bug :). Mail will be forgotten, bug entries are less likely to be lost. Philippe |