pointer to a pointer for the last argument of pthread_create() caused crash

Question

Just came across a weird problem regarding how to pass the 4th argument to pthread_create().

Originally, I wrote the code as follows:

auditLogEntry *newEntry = NULL;

// malloc and init the memory for newEntry
rc = audit_init_log_entry(&newEntry);
// wrapper of 'goto cleanup'
ERR_IF( rc != 0 );
...
rc2 = pthread_attr_init(&attr);
ERR_IF( rc2 != 0 );
rc2 = pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
ERR_IF( rc2 != 0 );
rc2 = pthread_create(&syslog_thread, &attr, syslog_thread_handler, (void *)newEntry);
ERR_IF( rc2 != 0 );
newEntry = NULL;
...
cleanup:
pthread_attr_destroy(&attr);
if (newEntry != NULL)
{
    audit_free_log_entry(newEntry);
    newEntry = NULL;
}

static void *syslog_thread_handler(void *t)
{
    auditLogEntry *entry = (auditLogEntry *)t;
    ... // code using entry
    cleanup:
    audit_free_log_entry(entry);

    pthread_exit(0);
}

Everything works fine.

Then, I made a change as:

rc2 = pthread_create(&syslog_thread, &attr, syslog_thread_handler, (void *)&newEntry);
ERR_IF( rc2 != 0 );
...
cleanup:
pthread_attr_destroy(&attr);
if (rc != 0 && newEntry != NULL)
{
    audit_free_log_entry(newEntry);
    newEntry = NULL;
}

static void *syslog_thread_handler(void *t)
{
    auditLogEntry **entry = (auditLogEntry **)t;
    ... // code using *entry
    cleanup:
    audit_free_log_entry(*entry);
    *entry = NULL;

    pthread_exit(0);
}

The thread handler would use *entry to access the log entry data, after the above change. But it didn't work. Worse, the process core dumped.

I tried 'man pthread_create', but found no special mentioning of how the last argument should be passed to it.

Any wrong-doing of mine, here?

so why change it? It worked ok in the first instance and that is a sensible way of doing things. What was the reason to change it if it worked? — mathematician1975
@mathematician1975: I want to reclaim the memory inside the thread handler, and set *entry to NULL after that. So the parent process would not have a danger of double-free the entry pointer. — Qiang Xu
@Qiang Xu Just do newEntry = NULL; right after the pthread_create call, and have your syslog_thread_handler thread free the memory. — nos
@nos: Yeah, that's what was figured out later to avoid a double free. I just didn't recognize there was an out-of-scope error in my code, taking for granted that a memory allocated in the heap would always be effective till the next free and even so for the stack variable itself! Thanks a lot for illustrating to me a pointer has two meanings - I usually only look at one side of it. — Qiang Xu

nos nos · Accepted Answer · 2012-08-08T21:48:15

You don't show the full code, so it's hard to tell what's going on.

However, &newEntry gives you a pointer to a variable on the stack. If newEntry goes out of scope, e.g. because your function ends, your other thread now has an invalid pointer - pointing to a place on the stack that is now gone. And dereferencing such a pointer causes undefined behavior.

int *foo(void) 
{
   int x = 2;
   return &x;
}

void bar(void)
{
   int *x = foo();
   printf("%d\n", *x); //can't do this, x points to something
                       //on the stack in the foo function, 
                       //which isn't valid any more
}

You'll have the same problem if the foo() function passed &x to a thread it created.

void foo(void) 
{
   int x = 2;
  ...
  pthread_create(&tid, bar, NULL, &x);
}

void *bar(void *arg) { int *x = arg;

   printf("%d\n", *x); //same problem here, x points to something
                       //on the stack in the foo function, which isn't valid 
                       //if the foo function ends. This concept is exactly the same
                       //if x had been a pointer inside the foo() function.
}

Here's perhaps your scenario:

int globalx = 2; //global variable
void foo(void) 
{
  int *x = malloc(sizeof(int));
  ...

  pthread_create(&tid, bar, NULL, &x); //we're still taking the address of 
                                       //a local variable. 
}

void *bar(void *arg) 
{
   int **x = arg;
   printf("%p\n", *x); //still the same problem here, x points to something
                       //on the stack in the foo function, which isn't valid 
}

pointer to a pointer for the last argument of pthread_create() caused crash

2 Answers