[Bug malloc/16742] New: race condition: pthread_atfork() called before first malloc() results in unexpected locking behaviour/deadlocks

["prumpf at gmail dot com" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=16742

            Bug ID: 16742
           Summary: race condition: pthread_atfork() called before first
                    malloc() results in unexpected locking
                    behaviour/deadlocks
           Product: glibc
           Version: 2.19
            Status: NEW
          Severity: normal
          Priority: P2
         Component: malloc
          Assignee: unassigned at sourceware dot org
          Reporter: prumpf at gmail dot com

Created attachment 7488
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7488&action=edit
Test case

This issue appears with pthreads and NPTL on the latest git version of glibc
(and the latest git version of Perl).

I'm not sure this is a bug according to the specifications, but it is both
extremely counterintuitive and occurs in practice, with the latest version of
Perl: https://rt.perl.org/Public/Bug/Display.html?id=121490  I believe it is
worth fixing.

When pthread_atfork() is called prior to the first malloc() call in the
program, the malloc atfork handlers are called before the user's atfork
handlers, resulting in unpredictable locking order and deadlocks.

Here's a test case:

#include <pthread.h>
#include <unistd.h>
#include <stdlib.h>

pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

void lock(void)
{
  pthread_mutex_lock(&mutex);
}

void unlock(void)
{
  pthread_mutex_unlock(&mutex);
}

void *lock_then_malloc(void *dummy)
{
  pthread_mutex_lock(&mutex);
  sleep(2);
  volatile void *throwaway = malloc(1024);
  pthread_mutex_unlock(&mutex);
  return NULL;
}

int main(int argc, char **argv)
{
  pthread_attr_t attr;
  pthread_t tid;

  volatile void *throwaway;
  if (argc > 1)
    throwaway = malloc(1024);

  pthread_atfork(lock, unlock, unlock);

  pthread_attr_init(&attr);
  pthread_create(&tid, &attr, lock_then_malloc, NULL);

  sleep(1);
  fork();

  return 0;
}


The expected behaviour is that the program terminates (after two seconds)
whether or not an extra argument is supplied; the actual behaviour is that the
program terminates only if an extra argument is present: the (otherwise
useless) call to malloc() means ptmalloc_init() gets called before rather than
after main() calls pthread_atfork(), changing the order in which atfork
handlers are run. In the case without an extra argument, the main thread then
holds the mutex and is waiting on malloc/arena.c's list_lock, while the other
thread hold list_lock and is waiting on the mutex.

This is essentially what Perl does: it calls pthread_atfork() before first
calling malloc(), then locks a mutex both in the atfork handler and in another
thread which then calls malloc().

While I think this definitely needs to be fixed, I'm not sure what the best fix
would be. A simplistic workaround would be to call malloc() from
pthread_create(), while a more elegant solution would be to allow a priority
argument for pthread_atfork(), with user-defined handlers always ending up
outside of malloc/glibc-defined handlers. The ChangeLog suggests there once
existed a function called __register_atfork_malloc(), which might have done
this already.

(Note that we never get to unlock(), so it's irrelevant whether
pthread_mutex_unlock() works after the fork(). )

-- 
You are receiving this mail because:
You are on the CC list for the bug.