Re: [Valgrind-users] FW: Helgrind to-do list

[David F. <fa...@kd...>]

On Friday 27 September 2013 17:51:19 Phil Longstaff wrote:
> > From: David Faure [mailto:fa...@kd...]
> > Sent: Friday, September 27, 2013 11:18 AM
> > To: Phil Longstaff
> > Cc: val...@li...
> > Subject: Re: [Valgrind-users] FW: Helgrind to-do list
> > 
> > On Friday 27 September 2013 15:01:54 Phil Longstaff wrote:
> > > I was thinking about this one last night, and it's trickier than I
> > > first thought.
> > > 
> > > L = lock, T = trylock
> > > Thread1: L1 L2
> > > Thread2: L2 T1
> > > 
> > > Not a deadlock because the trylock will just fail.  However, suppose
> > > we
> > > have:
> > > 
> > > Thread1: L1 L2
> > > Thread2: L2 T1
> > > 
> > > And then later:
> > > 
> > > Thread 3: L1 L2
> > > 
> > > When helgrind handles L2, it would already find the graph edge L1 ->
> > > L2 so wouldn't it just return since that is the "correct order"?
> > > David sent me some past e-mail and I saw some comments about putting
> > > lock vs trylock into the graph.  Seems to me that when processing T2,
> > > helgrind would not report a problem, but would add the T2 -> L1 link,
> > > and would also need to ensure that if L1 -> L2 happens in the future, it
> > > is reported.> 
> > A failing trylock cannot create a dead lock.
> > Only a succesful one, can.
> > 
> > So the question is whether we can assume a failed trylock could have
> > succeeded in creating a deadlock if it hadn't failed. In your particular
> > example, we can.
> > In many other cases, we can't know that "what happened after the failed
> > trylock" would have happened too, if it had succeded. There could be an
> > if() statement :-)
> > 
> > So IMHO it's much easier to just drop failed trylocks and only remember
> > successful ones, but yes, one can refine that for the case above, i.e.
> > when the failed-trylock is the last thing in the chain.
> > 
> > If anything happens *after* a failed trylock, then one can't store a
> > "T1 -> anything" link.
> 
> I agree.  My question is what we should store after a *successful* trylock.
> 
> Suppose helgrind sees these threads:
> 
> Thr1: L1   L2
> 
> So, it should add L1 -> L2 to the graph
> 
> Thr2: L1  L2
> 
> Assume thr1 has unlocked both L1 and L2.  Since L1 -> L2 already exists,
> does helgrind do anything?

Surely not, since L1 -> L2 is already there?

> Thr3: L2  T1 (unsuccessful)
> 
> No deadlock can occur, no edge should be added, no report.

Right.

> Thr3: L2 T1 (successful)
> 
> So, is your suggestion that L2 -> L1 should be added here, should be
> indistinguishable from L1 -> L2 added previously, and that the report
> should happen here, even though this operation would not be the one that
> causes the deadlock?

You're right. The whole point of trylock is to not deadlock :)
What thread 3 is doing, is valid.

The difference between "L2 L1" and "L2 T1(successful)" is that the first one 
should lead to a report and the second one shouldn't.
But once that step is done, in both cases we want L2 -> L1 in the graph.

So that
Thr 1: L2 T1 (successful)
Thr 2: L1 L2 (after thread 1)
gives a report of wrong lock order.

-- 
David Faure, fa...@kd..., http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5