helgrind in valgrind complains with simple mutex

Question

Im debugging some threaded code, and was using valgrind --tool=helgrind and for some reason helgrind does not like the simple example below.

Before I start a thread I lock the mutex. In the end of the thread I'm unlocking, and thereby making sure that only one thread can run at a time, by assuming that the mutex will be locked until the thread is finished.

Why is this not valid according to valgrind?

This is a part of a bigger program, where my main program is reading/parsing data it will launch an analysis thread, but I only want one analysis thread running at a time.

#include <pthread.h>
#include <stdio.h>

pthread_mutex_t mutex;

void *inner(void *ptr){
  size_t threadid=(size_t)ptr;
  int sleepval = lrand48() % 5 +1;
  fprintf(stderr,"thread: %lu will wait:%d\n",threadid,sleepval);fflush(stderr);  
  sleep(sleepval);
  pthread_mutex_unlock(&mutex);
}




int outer(size_t ntimes){
  pthread_t thread1;
  size_t i;
  for(i=0;i<ntimes;i++){
    pthread_mutex_lock(&mutex);
    if(pthread_create( &thread1, NULL, inner, (void*) i))
      fprintf(stderr,"Problems creating thread\n");
  }
    pthread_mutex_lock(&mutex);
    pthread_mutex_unlock(&mutex);
}


int main(){
  pthread_mutex_init(&mutex, NULL);
  outer(3);
  return 0;
}

==8326== Helgrind, a thread error detector
==8326== Copyright (C) 2007-2013, and GNU GPL'd, by OpenWorks LLP et al.
==8326== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==8326== Command: ./a.out
==8326== 
thread: 0 will wait:1
==8326== ---Thread-Announcement------------------------------------------
==8326== 
==8326== Thread #1 is the program's root thread
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #1: Attempt to re-lock a non-recursive lock I already hold
==8326==    at 0x4C32010: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400959: outer (threadTest.c:21)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326==  Lock was previously acquired
==8326==    at 0x4C32145: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400959: outer (threadTest.c:21)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== ---Thread-Announcement------------------------------------------
==8326== 
==8326== Thread #2 was created
==8326==    at 0x515543E: clone (clone.S:74)
==8326==    by 0x4E44199: do_clone.constprop.3 (createthread.c:75)
==8326==    by 0x4E458BA: pthread_create@@GLIBC_2.2.5 (createthread.c:245)
==8326==    by 0x4C30C90: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400976: outer (threadTest.c:22)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #2 unlocked lock at 0x6010A0 currently held by thread #1
==8326==    at 0x4C325C0: pthread_mutex_unlock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400937: inner (threadTest.c:11)
==8326==    by 0x4C30E26: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4E45181: start_thread (pthread_create.c:312)
==8326==    by 0x515547C: clone (clone.S:111)
==8326==   Lock at 0x6010A0 was first observed
==8326==    at 0x4C31DDA: pthread_mutex_init (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4009D0: main (threadTest.c:31)
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #1: Bug in libpthread: recursive write lock granted on mutex/wrlock which does not support recursion
==8326==    at 0x4C32145: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400959: outer (threadTest.c:21)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
thread: 1 will wait:4
==8326== ---Thread-Announcement------------------------------------------
==8326== 
==8326== Thread #3 was created
==8326==    at 0x515543E: clone (clone.S:74)
==8326==    by 0x4E44199: do_clone.constprop.3 (createthread.c:75)
==8326==    by 0x4E458BA: pthread_create@@GLIBC_2.2.5 (createthread.c:245)
==8326==    by 0x4C30C90: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400976: outer (threadTest.c:22)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #3 unlocked lock at 0x6010A0 currently held by thread #1
==8326==    at 0x4C325C0: pthread_mutex_unlock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400937: inner (threadTest.c:11)
==8326==    by 0x4C30E26: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4E45181: start_thread (pthread_create.c:312)
==8326==    by 0x515547C: clone (clone.S:111)
==8326==   Lock at 0x6010A0 was first observed
==8326==    at 0x4C31DDA: pthread_mutex_init (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4009D0: main (threadTest.c:31)
==8326== 
thread: 2 will wait:1
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #1: Attempt to re-lock a non-recursive lock I already hold
==8326==    at 0x4C32010: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4009B1: outer (threadTest.c:25)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326==  Lock was previously acquired
==8326==    at 0x4C32145: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400959: outer (threadTest.c:21)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== ---Thread-Announcement------------------------------------------
==8326== 
==8326== Thread #4 was created
==8326==    at 0x515543E: clone (clone.S:74)
==8326==    by 0x4E44199: do_clone.constprop.3 (createthread.c:75)
==8326==    by 0x4E458BA: pthread_create@@GLIBC_2.2.5 (createthread.c:245)
==8326==    by 0x4C30C90: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400976: outer (threadTest.c:22)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #4 unlocked lock at 0x6010A0 currently held by thread #1
==8326==    at 0x4C325C0: pthread_mutex_unlock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x400937: inner (threadTest.c:11)
==8326==    by 0x4C30E26: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4E45181: start_thread (pthread_create.c:312)
==8326==    by 0x515547C: clone (clone.S:111)
==8326==   Lock at 0x6010A0 was first observed
==8326==    at 0x4C31DDA: pthread_mutex_init (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4009D0: main (threadTest.c:31)
==8326== 
==8326== ----------------------------------------------------------------
==8326== 
==8326== Thread #1: Bug in libpthread: recursive write lock granted on mutex/wrlock which does not support recursion
==8326==    at 0x4C32145: pthread_mutex_lock (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
==8326==    by 0x4009B1: outer (threadTest.c:25)
==8326==    by 0x4009DA: main (threadTest.c:32)
==8326== 
==8326== 
==8326== For counts of detected and suppressed errors, rerun with: -v
==8326== Use --history-level=approx or =none to gain increased speed, at
==8326== the cost of reduced accuracy of conflicting-access information
==8326== ERROR SUMMARY: 9 errors from 7 contexts (suppressed: 104 from 61)

Answer 1

The immediate issue is that you lock a mutex in one thread and unlock it in a different thread. You also try to lock the same non-recursive mutex twice in the same thread. Since only the thread which acquired the lock can release it that would be a dead-lock.

To achieve the semantic you want you probably want you can just join the newly created thread: the join will block until the corresponding thread exits.

Alternatively, you can protect an active thread count which is incremented by the creating thread and decremented by when the thread finishes. The finishing thread would also signal a condition variable. The creating thread would check the count and wait on the condition variable if the count is too high.

Having a processor thread that uses and potentially blocks on a job queue is probably the easiest and most effective approach to limit the number of worker threads.

Answer 2

From the man page of pthread_mutex_lock:

If a thread attempts to unlock a mutex that it has not locked or a mutex which is unlocked, undefined behavior results.

So you have to lock and unlock a mutex in the same thread.

Also your functions should return a value, which matches the prototype.

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

pthread_mutex_t mutex;

void *inner(void *ptr){
  size_t threadid = *(size_t *)ptr;
  int sleepval = rand() % 5 +1;
  pthread_mutex_lock(&mutex);
  fprintf(stderr,"thread: %lu will wait:%d\n",threadid,sleepval);
  fflush(stderr);
  sleep(sleepval);
  pthread_mutex_unlock(&mutex);
  return NULL;
}

int outer(size_t ntimes){
  pthread_t thread[ntimes];
  size_t i, id[ntimes];
  for(i=0;i<ntimes;i++){
    id[i] = i;
    if(pthread_create(thread + i, NULL, inner, &id[i]))
      fprintf(stderr,"Problems creating thread\n");
  }

  for(i=0;i<ntimes;i++)
    pthread_join(thread[i], NULL);
  return 0;
}

int main(void){

  pthread_mutex_init(&mutex, NULL);
  outer(3);
  return 0;
}

This works for me. I also added the missing headers and changed the function parameter to match the new requirements.

A side note: The outer function uses variable length arrays now, so it is only working in C99, not in C++ (The question is tagged as both).