Can't access struct elements from a struct passed into a pthread

Question

I am stuck on this and cannot seem to figure out what is going on. For background, I am trying to pass a struct into 3 pthreads, then modify that struct in the threads, then store back each struct into the main thread, and add up the partial results gotten by each one in order to speed up this part of my code that seems to be a main bottleneck in performance. I have used code very similar to this in another project written in C, but when I copied it into this C++ program and modified it to fit this problem, I have run into some weird errors.

First of all, I had to make some casts that I didn't have to make in C in order to get it to compile. Since I included the thread_work function in the class as a method, it saw the type as (void * (AIShell::*)(void *), so I needed to cast it as (void * (*)(void *), which allows it to compile and run, but it gives me a compiler warning. The reason I included it as a method rather than just a function is so that I can call another class method from within this one, so it is required. Is there any way I should revise my code to make this go away, or would this just be an unfortunate side effect of using my thread function in a class?

AIShell.cpp: In member function ‘double AIShell::heuristic(int**)’:
AIShell.cpp:173:78: warning: converting from ‘void* (AIShell::*)(void*)’ to ‘void* (*)(void*)’ [-Wpmf-conversions]
   th = pthread_create(&new_threads[i], &attr, (void * (*)(void *)) &AIShell::thread_work, (void *) (se+i));

Secondly, and most importantly I get a segfault when running my code. I have narrowed down where the problem happens, and it happens in my thread_work function whenever I try to access a member of the struct that I passed into the thread (ie doing se->start_count):

125 struct start_end
126 {
127     int ** board;
128     int start_col;
129     int end_col;
130     int start_row;
131     int end_row;
132     double total;
133 };
134
135 void * AIShell::thread_work(void * arg)
136 {
137     struct start_end * se = (struct start_end *) malloc(sizeof(struct start_end));
138     se = (struct start_end *) arg;
139     printf("\nse->total = %lg; se->start_row = %d; se->start_col = %d\n\n", se->total, se->start_row, se->start_c    ol);
140     fflush(stdout);
141     for (int row = se->start_row; row != se->end_row; row++)
142     {
143         for (int col = se->start_col; col != se->end_col; col++)
144         {
145             se->total += check8(se->board, col, row);
146         }
147     }
148     printf("asdfasdfasfdasdfasdfasdfasdfasdfasdfasdf\n");
149     fflush(stdout);
150     pthread_exit((void *)se);
151 }

When it gets to this part of the code, which is what is run by each thread, it initially failed on line 141 where the for loop started, but after adding the print statement on line 139, it fails there, which confirms that the problem is with referencing the struct members.

However, if I add an "&" before "arg" in line 138 so that it looks like this:

138     se = (struct start_end *) &arg;

it doesn't segfault anymore, but it gets garbage values which breaks some of my code later on. To my knowledge I am properly assigning values to the structs before they are passed in to the thread, so I am not really sure what is going on here.

This code is the method that acts as the "main thread" which initializes the structs, spawns the other threads, and is supposed to get back the results from each one and add them up to get the final result. It also acts as a thread for this itself so I used a similar code segment here that I did in the above function so I would be using all of my resources ;)

153 double AIShell::heuristic(int ** board)
154 {
155     pthread_t new_threads[3] = {0};
156     pthread_attr_t attr;
157     int th;
158     long t;
159     struct start_end * se;
160     se = (struct start_end *) malloc(3*sizeof(struct start_end));
161
162     pthread_attr_init(&attr);
163     pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
164
165     for (int i = 0; i <3; i ++)
166     {
167         (se+i)->board = copy(board);
168         (se+i)->start_row = numRows/4*i;
169         (se+i)->end_row = numRows/4*(i+1);
170         (se+i)->start_col = numCols/4*i;
171         (se+i)->end_col = numCols/4*(i+1);
172         (se+i)->total = 0;
173         th = pthread_create(&new_threads[i], &attr, (void * (*)(void *)) &AIShell::thread_work, (void *) (se+i));
174         if (th)
175         {
176             perror("pthread_create");
177             exit(1);
178         }
179     }
180
181
182     int start_row = numRows/4*3;
183     int end_row = numRows;
184     int start_col = numCols/4*3;
185     int end_col = numCols;
186     int total = 0;
187
188     for (int row = start_row; row != end_row; row++)
189     {
190         for (int col = start_col; col != end_col; col++)
191         {
192             total += check8(board, col, row);
193         }
194     }
195
196     for (int i = 0; i != 3; i++)
197     {
198         th = pthread_join(new_threads[i], (void **) (se+i));
199         if (th)
200         {
201             perror("pthread_join");
202             exit(1);
203         }
204     }
205
206     for (int i = 0; i != 3; i++)
207     {
208         total += (se+i)->total;
209         free((se+i));
210     }
211
212     pthread_attr_destroy(&attr);
213
214     free(se);
215
216     return total;
217 }

I realize that doing it this way is very "C" of me and I could be doing it with "new" or whatever, but as I said, I adapted the code from a "C" project to use in this "C++" project, and would like to do it this way without having to re-write everything. I also am not positive I am "freeing" correctly for this, but it doesn't even get there in my code yet, so that is more of a secondary issue here. So with all that out of the way, does anyone see what it is that I am doing wrong here? I have tried so many things, and just can't seem to get it to work :/

Answer 1

You need to fix that warning.

Your case is a little different but there's an easy way. Create a little container object that looks like this

class Context { AIShell* theObject; int number; }

Create a static function that looks like this

static void* ThreadFunction(void* context)

Then, where you are currently calling pthread_create, instantiate a Context object with "this" and the number (1 through 3).

Pass ThreadFunction as the start_routine, and the Context object as the last arg. Then, within ThreadFunction, you can call

Context* ctxt = (Context*) arg; ctxt->theObject->thread_work(ctxt->num)

Answer 2

You're leaking memory there, for one. you malloc memory and assign pointer to se in thread function, then assign value of arg to the same pointer. Memory , allocated by first line of thread_work will be lost.

Second, why you cast so (void * (*)(void *)) &AIShell::thread_work?

Function declared as

void * AIShell::thread_work(void * arg)

doesn't require such cast unless.. it's a method of class and AIShell is class, not namespace. Is AIShell a class then? In that case you either need a static function or use std::bind to generate a wrapper for it, non-static methods actually have hidden argument of "this".

Line

se = (struct start_end *) &arg;

is wrong because you get address of argument arg(which is in stack supposedly?) and convert it to pointer to struct, while actually that memory contains pointer. It doesn't produce segfault because most likely you appear to access stack or data segment, no violation there. Debug program and check value of arg in worker and compare it to value of se+i given to pthread_create. That is value of pointer in first place.