Segmentation fault for threads

[zizou86]

 //Written for Ubuntun 9.10 using g++ in Netbeans 6.5.1
Header file:
class threader {
public:
    threader();
    threader(const threader& orig);
    virtual ~threader();
    int createThread(MyNameSpace::Puzzle *mz1;

private:
    pthread_t thread, thread2, thread3, thread4, thread5, timerThread;
    
};


#include statements, some constructors not included. Too many.
MyNameSpace::Puzzle *mz; //some global variables required as a shared resource //for threads

threader::threader() {
}

bool hitEnd = false;

void *maze_thread (void *arg)
{
   //HUGE CHUNK OF CODES I TOOK OUT. Did what the threader is supposed to do. 
   //Did not seem to have a problem after doing some rather extensive debugging.
}

int threader::createThread(MyNameSpace::Puzzle *mz1)
{
    int status, status2, status3, statusTime;
    mz = (MyNameSpace::Puzzle*)malloc (sizeof (MyNameSpace::Puzzle));
    mz = mz1;

    void* statusJoin;
    void* statusJoin2;
    void* statusJoin3;
    void* statusJoin4;
    void* statusJoin5;
    VectorOfVectorOfPointStructType allPaths;
    char* name1 = "ThreadOne";
    char* name2 = "ThreadTwo";
    char* name3 = "ThreadThree";
    char* name4 = "ThreadFour";
    char* name5 = "ThreadFive";
    char* name6 = "ThreadThree";
    
    createCurrMaze();
    int i = 0;
    bool cont = true;

    while (cont == true)
    {
        //std::cout << "hehe6\n";
        status = pthread_create (&thread, NULL, maze_thread, name1);
        status2 = pthread_create (&thread2, NULL, maze_thread, name2);
        status3 = pthread_create (&thread3, NULL, maze_thread, name3);
        status3 = pthread_create (&thread4, NULL, maze_thread, name4);
        status3 = pthread_create (&thread5, NULL, maze_thread, name5);
        //status = pthread_create (&thread, NULL, maze_thread, NULL);

        //pthread_join(thread4, &statusJoin);
        //pthread_join(thread5, &statusJoin);

        //std::cout << "hehe4\n";
        pthread_join(thread, &statusJoin);
        //std::cout << "hehe5\n"; //such cout << hehe lines are for debugging.
        pthread_join(thread2, &statusJoin2);
        
        pthread_join(thread3, &statusJoin3);
        pthread_join(thread4, &statusJoin4);
        pthread_join(thread5, &statusJoin5);
        
        i++;

        if (hitEnd == true) //a global var. maze_thread sets to true after //solving puzzle.
        {
            std::cout << "End found!\n";
            cont = false;
            break;
        }
    }
}

Hi, referring to codes above, all codes pasted lie in Threader class. Only createThread() function belong to threader namespace, the rest are outside of the namespace. I could not get the maze_thread and all other threads it wants to call to belong to a namespace, hence I took them all out of the namespace and header files. My apologies that I cannot include the full code as it is really long with over 600 lines of codes. Not the best of designs.

Basically, main() will call createThread(mz1) to start threading. createCurrMaze(); is a function to initialize the initial puzzle, you know reading in files and the like. This function does not seem to have any problem and hence I did not include it in.

createThread will then create a total of 5 threads which will act independently and run around the puzzle, trying to solve it.

The problem comes when some of them need to be killed when they hit upon dangerous artifacts in the puzzle. When they are killed, control falls back to createThread() again to rethread the same thread from the beginning. This continues until the end is found.

This process of killing and rethreading is when I might get a segmentation fault. The seg fault usually pops up after the threads had already ran through quite a few times, sometimes a few thousands before it occur. I "killed" the thread by letting maze_thread (void *arg) function end its while loop and then returning a NULL, and createThread(MyNameSpace::Puzzle *mz1) will rethread again.

As you noticed, I dropped some numbered std::cout << "hehe" statements throughout the program so as to know where it got stuck at for the seg fault. This is rather rudimentary debugging but the threads will restart thousands of times and hence I cannot use the debugger to loop through it thousand times.

It seems to me the problem mostly lie in createThread(mz1), in the while loop. As to the exact line, I am not too sure as it never seemed to be the same.

It also seem strange to me, that when I took out all the std::cout << "hehe", the seg fault seem to occur much less frequently. When I overload the program with chunks of cout, it seem to crash nearly everytime. When I took them out, it seem much better, probably crashing once every 5 times.

Is there anything I am missing in my threading? I suspect there is something to do with pointers passed to the threads getting mixed up and becoming uninitialised.

Please assist. I am really at my wits end.

Thank you.

[PanGalactic]

Where is the matching free() for the malloc() on 32?

What are you really trying to do on line 33?

[zizou86]

Hi PanGalactic,

Thank you for your prompt reply.

I hate to say this, but I have no idea really. I know that it does some form of memory allocation. What this thing does was, there are 2 Puzzles. One is the original puzzle that has all the necessary barriers and valid paths taken from a text file, this is sort of the "answer sheet". I will have yet another puzzle that is initially blank and my threads will start moving around it.

main() will call another class not listed here to take in the text file and convert it into a Puzzle object, and then main() will call createThread(MyNameSpace::Puzzle *mz1) and pass in the Puzzle object as mz1.

After this, createThread will do the necessary setting up of the current puzzle (the blank puzzle for exploring purposes). Finally, createThread will then call maze_thread (void *arg) to start threading and exploring.

So, as you can see, I will have to allow maze_thread (void *arg) to be able to access both the blank current Puzzle and the original puzzle.

However, I had problems passing in mz1 (the input argument of createThread) to maze_thread (void *arg). Hence, I declared another global Puzzle object, mz, which will be a direct copy of mz1. This new object mz will then be accessible to all methods, including the threading ones, in the entire class. I have several more functions that I did not paste out which will access the Puzzles as well.

Hence, thats what lines 32 and 33 do. Do a copy of the Puzzle object so it will end up as a Global object accessible to all functions. As to line 32, I did some research on memory allocation and it seemed that it allocates memory for that object so it does not die out out on threads.

Please kindly enlighten me if I am wrong. In the meantime I will go research on freeing the memory up when back in Ubuntu.

[PanGalactic]

But mz is not a copy of mz1. They are both pointers. mz is a pointer to an uninitialized block of memory the same size as a Puzzle on line 32. Then it is changed to point to mz1 on line 33. The block of memory you just allocated no longer has a reference and is leaked.

If you have similar mistakes elsewhere in your code, it is no wonder you are segfaulting.

Also, I have no idea what the Puzzle class looks like or how the threads manipulate it. It is impossible to tell whether it is OK. The fact that it is not a const object is worrisome. Global, non-const objects are a big red flag when doing threaded programs.

There is so much wrong with this code as it is (using malloc for objects, memory leaks, pointer mistakes) that you might want to consider whether your are starting on threads too soon. Pointers and memory management require a lot of care in C++. Threads require even more attention to detail.

The other comment that I will make is that using Boost Threads is much easier than raw pthreads. I highly recommend using the Boost Thread library if you can.

[zizou86]

Hi PanGalactic,

Thank you for your help again.

As you said, I did some reading up and managed to change some parts. I am fully aware that my knowledge of malloc and pointers in c++ are near to zero (I have just picked up c++ for a month or so).

Now:

int status, status2, status3, statusTime;
    //mz = (Assignm2::Maze*)malloc (sizeof (Assignm2::Maze));
    mz = mz1;

I took this stupid line of malloc out and it is still crashing. After abit more research and tests with valgrind, I realised pthread_join seems to cause this mainly. Valgrind keeps throwing out a "read size of 4" error in pthread_join. Finally, I realised that

status = pthread_detach (pthread_self ());

in the threading function seems to not work with pthread_join and hence removed that line.

Now, it seems strange to me that valgrind is working fine with the program not crashing. No seg faults whatsoever after running it for more than 30 times. Can exit gracefully and all and valgrind returns no errors except for some little suppressed ones.

However, when I run it in netbeans or via terminal command line, it still does give a segfault 1 out of 3 times.

So this is to do with a de-referecing of pointer and letting the memory run loose? I had checked the program thoroughly and the only malloc line seems to be that.

These are the error messages returned when NOT running in valgrind (either via terminal or netbeans):

Thread No. 513 belonging to ThreadFour has been killed as it has encounted a monster.

ThreadThree started 

ThreadTwo started 
Thread No. 514 belonging to ThreadThree has been killed as it has encounted a monster.
*** glibc detected *** ./threader: malloc(): memory corruption: 0x08848a50 ***
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6[0x198ff1]
/lib/tls/i686/cmov/libc.so.6[0x19bbe3]
/lib/tls/i686/cmov/libc.so.6(__libc_malloc+0x58)[0x19d898]
/usr/lib/libstdc++.so.6(_Znwj+0x27)[0x53cbb7]
./threader[0x804d1cc]
./threader[0x804caca]
./threader[0x804c346]
./threader[0x804bc91]
./threader[0x804ef18]
Segmentation fault

Can you please put me to the right direction please. Please let me know if you need more information.

[zizou86]

After doing much clean-ups and research,

I have more or less cleaned up most seg-faults except for one very occasional one..

it seems to me that the "status = pthread_create (&thread, NULL, maze_thread, name1);" lines (lines 56 to 59) in the code are the culprint...

As you see, what my intention was to keep looping the threads and to "reborn" threads each time they have been killed until the Puzzle has been solved.

Now, from my understanding of pointers, it seems that each time the while loop repeats, I am doing a "rethread" and this re-thread causes the original &thread to dereference and reference on to the new instance of that thread.

There are 5 threads running at one time, name to name5. So each time all 5 dies, the while loop ensures that all 5 are reborn. The main program will wait for the 5 threads to die (via pthread_join). Hence, when all 5 dies and the program reloops to recreate, I guess the pointers for the previous 5 threads get dropped for the new 5 threads?

I have tried various delete, pthread_exit() and free() instructions but all of them seem to make it worse with double free errors. Please note that in the threading function maze_thread(void* arg), I do not have any codes related to thread creation of destruction. The last line of the function simply does a "return NULL".

Am I correct to say that this is probably my main problem?

[PanGalactic]

There is just not enough detail in the code you have posted to say with any confidence where the problem lies.