Re: [RFC][BZ #16159] Detecting that recursive mutex recursed.

[Rich Felker <dalias at aerifal dot cx>]

On Wed, Nov 13, 2013 at 08:03:56PM +0100, OndÅej BÃlka wrote:
> On Wed, Nov 13, 2013 at 01:12:57PM -0500, Rich Felker wrote:
> > On Wed, Nov 13, 2013 at 02:46:08PM +0100, Ondrej Bilka wrote:
> > > Hi,
> > > 
> > > To fix bugs with malloc async safety I would need a following primitive
> > > 
> > > int ptrhead_mutex_lock_cnt(pthread_mutex_t *m, int *cnt);
> > > 
> > > Which works like pthread_mutex_lock except it returns a number of times lock was recursively locked.
> > > This would allow us to detect that function was invoked in signal and provide safe workaround.
> > 
> > glibc's recursive mutexes are not reentrant, so this will not work.
> > You could detect the common case where the interrupted code had
> > already fully obtained the mutex, but not the race condition where
> > mutex acquisition itself was interrupted. (FYI I have a working
> > recursive+reentrant mutex implementation that avoids this issue.)
> >
> It would require to make mutex reentrant which is separate issue from
> proposal.

But it's a prerequisite for fixing this issue anyway.
pthread_mutex_lock_cnt cannot be expected to work on a mutex where the
acquire operation is not atomic (atomic in the sense of not being able
to be interrupted by a signal handler).

> Alternative solution that I wanted to avoid is to use additional
> thread-local variable to detect signals. A count would be maintained by
> atomic increment/decrement.
> 
> Do you know how to generate cmpxchgb instruction without lock prefix as
> there is no need of interthread synchronization?

Why do you need any atomicity or asm? In principle, read, modify,
write is perfectly valid for this usage as long as the write is atomic
(e.g. not using 2 16-bit writes to implement a 32-bit write). If
you're not happy assumin 32-bit writes are atomic, then yes, a little
more work would be required.

But I'd be more concerned about the cost of TLS access. Unfortunately
this also applies with recursive mutexes, since they need TLS access
to get their own thread id and record an owner. On some archs (older
MIPS where the instruction to get the thread pointer traps into
kernelspace, and probably some relevant ARM variants) reading the
thread pointer can be expensive, and I question whether this expense
can be justified in malloc. I'd rather just abandon aims to make
malloc AS-safe and focus on its performance, and have code that needs
malloc-like functionality with AS-safety use another interface.

Rich


P.S. glibc still officially supports user replacement of malloc, so
nothing in libc is justified in assuming properties of its own malloc
implementation like AS-safety.