0

This is a stripped-down version of some code that works perfectly fine as a single-threaded program. I am trying to make it multithreaded. The goal is to have a global list of structs, and, in a loop, reallocate memory for the list and extend its size by one struct for each iteration of the loop. I receive the following error:

Terminated due to signal: ABORT TRAP (6)
doublefree(54652,0x70000020a000) malloc: *** error for object 
0x7fbc13403400: double free

As mentioned, it works if the number of threads is one. I'm assuming the multiple threads are attempted to free the same memory during realloc(). But shouldn't the mutex prevent that? The pointer for the list is global, so shouldn't every thread see the changed pointer made by the others? I don't see how this is any different from the single-threaded version. If I run it over and over again, I can eventually get the correct output, so there is some race condition that I'm not seeing.

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

typedef struct { 
    char key[512];
    int value; 
} KV_Pair;
KV_Pair *pairList;
int count = 0;
void *doStuff(void *args);
pthread_mutex_t mutex;

int main(int argc, char *argv[]) {
    pairList = malloc(sizeof(KV_Pair));
    pthread_t threads[4];
    int i;
    for(i = 0; i < 4; i++) {
        pthread_create(&(threads[i]), NULL, doStuff, NULL); }
    for(i = 0; i < 4; i++) {
        pthread_join(threads[i], NULL); }
    free(pairList);
}
void *doStuff(void *args) {
    while (1) {
        pthread_mutex_lock(&mutex);
        count++;
        pairList = 
            realloc(pairList, sizeof(KV_Pair) * count);
        pthread_mutex_unlock(&mutex);
    }
    pthread_exit(0);
}
jmsul
  • 3
  • 1
  • The compiler might use a register to store `pairlist` until the function returns. Try declaring `pairList` as `volatile KV_Pair *pairList;` to force the compiler to write the new value back to memory immediately. – Klas Lindbäck Oct 17 '16 at 14:39
  • What happens when `realloc()` eventually fails and sets `pairlist` to `NULL`? – Andrew Henle Oct 17 '16 at 14:39
  • @KlasLindbäck - `pthread_mutex_[un]lock()` includes a memory barrier. See http://stackoverflow.com/questions/3208060/does-guarding-a-variable-with-a-pthread-mutex-guarantee-its-also-not-cached and http://stackoverflow.com/questions/24137964/does-pthread-mutex-lock-contains-memory-fence-instruction – Andrew Henle Oct 17 '16 at 14:41
  • 1
    @usr I'm not sure what's going on, hence it was a comment. The code as posted doesn't seem as if it should generate a double-free error. I'm wondering if `pthread_mutext_t mutex;` is technically an uninitialized mutex. Perhaps `pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;` would change the behavior. – Andrew Henle Oct 17 '16 at 14:45
  • @usr `PTHREAD_MUTEX_INITIALIZER` is not the same as a 0-initialized `pthread_mutex_t`. With OPs code `pthread_mutex_lock(&mutex);` invokes undefined behaviour. After initializing the mutex correct, the code works for me: http://ideone.com/wJVKCP – mch Oct 17 '16 at 15:11
  • @usr *it doesn't make a difference in glibc (and not POSIX)* That's because `PTHREAD_MUTEX_INITIALIZER` in glibc is nothing but zeros, so an uninitialized static `pthread_mutex_t` "works" under glibc. – Andrew Henle Oct 17 '16 at 15:31
  • @usr *Which is what I said?* I know. But ${RANDOM_INTERNET_READER} won't. And perhaps the question's poster *didn't*... – Andrew Henle Oct 17 '16 at 15:44

1 Answers1

1

You forgot to initialize your mutex:

pthread_mutex_init(&mutex, NULL);

So the locking wasn't happening. You would have seen this if you had checked the return code from:

pthread_mutex_lock(&mutex);

(22, invalid argument, instead of 0, success) rather than ignore it...

cdlane
  • 40,441
  • 5
  • 32
  • 81
  • Problem solved. You're a lifesaver. I can't believe that I forgot that I needed to initialize (I even used to use mutexes extensively)! – jmsul Oct 17 '16 at 19:06