[PATCH] Teach pthread checker about double unlocks

Daniel Fahlgren daniel at fahlgren.se
Wed Mar 19 13:17:26 PDT 2014


Hi Jordan,

On Wed, 2014-03-19 at 10:20 -0700, Jordan Rose wrote:
> Hi, Daniel. I'm wondering why we need a Locked state at all. We still
> want to support an unbalanced unlock, and if we want to handle
> recursive locks (PR10418) we'd have to do more work anyway than just
> "locked" or "unlocked". (The existence of recursive locks means that a
> double-unlock warning isn't always correct.)
> 
> 
> I didn't look at the patch too closely, but it mostly looks sensible.
> What do you think about this, though?
> Jordan

You are right. The locked state isn't needed since that information
already is in the list of current locks. But we must know if the lock is
unlocked or not for the double detection to work, removing the entry
from the map when locking it would do.

I'll go back to the drawing board and try to think of a way to address
the recursive locks while at it. What is your opinion about default
behavior regarding recursive locks?

int main(int argc, char *argv[])
{
  pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER;
  pthread_mutex_lock(&mtx);
  pthread_mutex_lock(&mtx);
  return 0;
}

will behave differently depending on the OS. On FreeBSD and Solaris the
process will terminate while Linux and AIX will deadlock. How OS X
behaves I do not know. Should the default be to allow recursive locking,
not allow it, or depend on the current OS?

Btw, what do you mean with "to support an unbalanced unlock"?

> On Mar 13, 2014, at 16:32 , Daniel Fahlgren <daniel at fahlgren.se>
> wrote:
> 
> > Hi Jordan,
> > 
> > This patch makes the pthread checker detect double unlocks. I have
> > copied the idea with a map from the stream checker, not sure if that
> > is
> > the best approach?
> > 
> > Tracking the state will later make it possible to warn if you lock a
> > destroyed mutex, calls pthread_init twice etc. But I haven't had
> > time
> > for that yet.
> > 
> > Best regards,
> > Daniel Fahlgren
> > <pthread.diff>

Cheers,
Daniel
> 





More information about the cfe-commits mailing list