2

I want to do something like this:

void *do_work_son(void *data)
{            
    mystruct *d = (mystruct*)data;
    while(true)
    {
        // d->whatever is corrupt
    }
}

void start_thread(pthread_t *mt)
{
    mystruct data = ...;
    pthread_create(&(*mt), NULL, do_work_son, (void *)&data);
}

int main()
{
    pthread mt;
    start_thread(&mt);
    // do more stuff here
    pthread_join(mt, NULL);
}

The idea is spawn off some threads and keep doing more work in main... then when done doing more work, wait for the threads to finish.

It compiles fine, but the data struct is corrupted when it gets accessed inside do_work_son. I think this is because the threads are exiting, even though I'm calling join in main. If I move the pthread_{create,join} calls both to start_thread, it works fine, but then my main function is blocked by the while loop. Am I crazy for doing it this way?

2 Answers2

5

I think this is because the threads are exiting,

No, that's because data is an automatic variable in the start_thread() function, which is invalidated when start_thread() returns, so using its address afterwards invokes undefined behavior.

Either malloc()ate some memory for it, or make it static, or whatever. Just make sure it survives the death of start_thread().

  • 1
    I type too slow :( Off topic: I liked your old photo better ;) – Daniel Fischer Mar 14 '13 at 21:17
  • @DanielFischer I've been typing *a lot* these days (implementing a scripting language). To the photo: I wished *that certain "someone"* will like it. Too bad she didn't... –  Mar 14 '13 at 21:19
  • @DanielFischer But it will persist nevertheless, maybe there are people who are wondering who the heck this H2CO3 guy is. :P –  Mar 14 '13 at 21:22
  • @H2CO3 Are you saying that the `malloc` can occur in `start_thread` and it will survive after scope leaves `start_thread`? –  Mar 14 '13 at 21:27
  • @H2CO3 One additional question: is this one specific case of the correct use of `malloc`? Can you give me a litmus test for when the use of `malloc` is appropriate over other methods? Thanks –  Mar 14 '13 at 21:45
  • @g33kz0r I was not referring to the use case. I was referring to the fact that Valeri did **not** cast its return value and used `sizeof(*pointer)` instead of the dangerous `sizeof(typename)`. –  Mar 14 '13 at 21:46
  • What I meant is, as a C programmer, when do I know the correct time to use `malloc`? –  Mar 14 '13 at 21:47
  • @g33kz0r Never, unless needed. –  Mar 14 '13 at 21:48
  • Lol. You're just being short because I accepted val's answer instead of yours. `sizeof(*pointer)` seems circular when that pointer is being defined by this statement. –  Mar 14 '13 at 21:48
4

The answer is simple: you are passing a reference of a local variable and then leaving the scope. Replace it with data allocated on heap and it would work like a charm

void start_thread(pthread_t *mt)
{
    mystruct *data = malloc(sizeof(*data));
    ...;
    pthread_create(mt, NULL, do_work_son, data);
}

EDIT:

About preassignment question:

void start_thread(pthread_t *mt)
{
    mystruct local_data = {...};
    mystruct *data = malloc(sizeof(*data));
    *data = local_data;
    ...;
    pthread_create(mt, NULL, do_work_son, data);
}
Valeri Atamaniouk
  • 5,125
  • 2
  • 16
  • 18
  • 1
    Wow, you just used `malloc()` **correctly.** Rare diamond nowadays on SO. –  Mar 14 '13 at 21:27
  • Is there a way to initialize mystruct with values as well as do the malloc all in one step? –  Mar 14 '13 at 21:29
  • @g33kz0r in two, if your data is POD: you can always make an assignment from some pre-created structure (static, local, other): `*data = my_preset_data`. – Valeri Atamaniouk Mar 14 '13 at 21:35
  • My code works now, thanks. I think you should change `sizeof(*data)` to `sizeof(data)`. –  Mar 14 '13 at 21:40
  • @g33kz0r Surprise, surprise :P –  Mar 14 '13 at 21:45
  • I guess reputation *is* everything :) –  Mar 14 '13 at 21:47
  • @g33kz0r Too bad C is not as popular as its sharp version or even JavaScript, I'm envious of the 100k elite :P Seriously, it **isn't.** Valeri just gave an answer that's free of all errors a typical 400-rep user would have made. Hats off. –  Mar 14 '13 at 21:50
  • 1
    I'm more of a python guy myself. Came back to C this week intentionally as an exercise in forcing myself to take computing seriously again. –  Mar 14 '13 at 21:51
  • 1
    @g33kz0r Now that attitude itself is worth an upvote to your comment. –  Mar 14 '13 at 21:52
  • Thanks, your help is sincerely appreciated. –  Mar 14 '13 at 21:52
  • @g33kz0r Welcome. I heartly recommend re-reading a good C tutorial or K&R if your intentions are serious. –  Mar 14 '13 at 21:53