Home
| Calendar
| Mail Lists
| List Archives
| Desktop SIG
| Hardware Hacking SIG
Wiki | Flickr | PicasaWeb | Video | Maps & Directions | Installfests | Keysignings Linux Cafe | Meeting Notes | Linux Links | Bling | About BLU |
I think your problem is here. I'll simplify; Afunc(void *idata) { struct foo *data; data = (struct foo *)idata; : free(data); /* This is a segfault*/ } void *data; data = malloc(...); Afunc(&data); Now, here is the way it should be: Afunc(void *idata) { struct foo *data; data = (struct foo *)*idata; : free(data); /* This woks nicely but ...*/ } Since you are passing in a pointer to a pointer (double indirect), your assignment is wrong: data = (struct spawnData_s *)obj; What you are assigning is a (struct spawnData_s **) not a (struct spawnData_s *). Actually, your code has several major problems (I assume that threadSpawn is called for each thread you want to start). 1. don't use &data as your argument, just use data. The reason is that in threadSpawn, data is an automatic variable and it's memor is deallocated when the function returns. If the child thread continues to use that pointer, several things could happen. Another function in the parent is invoked and ovewrites that memory or second call to threadSpawn overwrites it before your child thread finished using it. This is a major bug. 2. The use of ThreadPool should be protected by a mutex. (I assume that ThreadPool is initialized properly.(This is race condition). ThreadPool[idx].Status is tested in threadSpawn. What if the child thread should be switched before the child has a chance to set the Status. You need to make sure you set the status BEFORE main or any other thread tests it. I suggest you make 2 corrections to your program: 1. add an element to ThreadPool for spawnData_s, then in threadSpawn, ThreadPool[idx].data = (spawnData_s *)malloc(....). 2. pthread_create(&ThreadPool[idx].aThread, NULL, (void *)funcWrapper, (void *)idx); 3. change void funcWrapper(void *obj) { struct spawnData_s *data; int idx; data = (struct spawnData_s *)obj; to: void funcWrapper(void *obj) { > struct spawnData_s *data; > int idx; /* You should pthread_mutex_lock here */ idx = (int)obj; ThreadPool[idx].Status = tp_InUse; data = ThreadPool[idx].data; /* You may pthread_mutex_unlock here (or before setting data)*/ And in threadSpawn: /* pthread_mutex_lock */ if (ThreadPool[idx].Status == tp_Available) { ThreadPool[idx].data = malloc(...). /* pthread_mutex_lock */ pthread_create(&ThreadPool[idx].aThread, NULL, (void *)funcWrapper, (void *)idx); Actually, this leaves a very slight open window, so I would add a ThreadPool[idx].Status = InProcess; before unlocking the mutex in the parent. Much simpler. You eliminate the loop inside the thread, you eliminate the confusion about the double indirect. -- Jerry Feldman <gaf at blu.org> Associate Director Boston Linux and Unix user group http://www.blu.org PGP key id:C5061EA9 PGP Key fingerprint:053C 73EC 3AC1 5C44 3E14 9245 FB00 3ED5 C506 1EA9
BLU is a member of BostonUserGroups | |
We also thank MIT for the use of their facilities. |