1
#include <pthread.h>
#include <stdio.h>
#include <unistd.h>
#include <vector>
#include <string>
#include <iostream>

pthread_mutex_t demoMutex         = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t  conditionVariable = PTHREAD_COND_INITIALIZER;
unsigned int    condition         = 0;

struct serverInfo
{
    unsigned int                  serverId;
    pthread_t                     threadId;
    std :: vector <std :: string> queue;
};
std :: vector <serverInfo> serverInfoVector;

void print_thread_id(pthread_t id)
{
    size_t i;
    for (i = sizeof(i); i; --i)
        printf("%02x", *(((unsigned char*) &id) + i - 1));
}

void * printHello (void* threadId)
{
    pthread_t *my_tid = (pthread_t *)threadId;

    pthread_mutex_lock (&demoMutex);

    while (condition == 0)
        pthread_cond_wait (&conditionVariable, &demoMutex);

    unsigned int i = 0;
    char         found = false;

    if (serverInfoVector.size () > 0)
    {
        while ((i < serverInfoVector.size ()) && (found == false))
        {
            if (*my_tid == serverInfoVector [i].threadId)
            {
                found = true;
                break;
            }
            else
                i++;
        }
    }

    while (!serverInfoVector [i].queue.empty ())
    {
        std :: cout << "\nThread: " << pthread_self () << ", poped from queue: " << serverInfoVector [i].queue.front ();
        serverInfoVector [i].queue.pop_back ();
    }

    pthread_mutex_unlock (&demoMutex);
    pthread_exit (NULL);
}

void checkServerExists (unsigned int serverNumber, std :: string message)
{
    unsigned int i     = 0;
    char         found = false;

    pthread_mutex_lock (&demoMutex);

    if (serverInfoVector.size () > 0)
    {
        while ((i < serverInfoVector.size ()) && (found == false))
        {
            if (serverNumber == serverInfoVector [i].serverId)
            {
                found = true;
                break;
            }
            else
                i++;
        }
    }

    if (found == false)
    {
        // This server doesn't exist, so create a thread for it, create a queue for it, push the message in the corresponding queue.
        // Push the server number in the serverNumberArray.

        // Create a thread for it.
        pthread_t newThread;
        int returnValue;
        if ((returnValue = pthread_create (&newThread,
                                    NULL,
                                    printHello,
                                    (void*) &newThread)) != 0)
        {
            printf ("\nerror: pthread_create failed with error number %d", returnValue);
        }
        printf ("\nIn checkServerExists ()`: thread id %ld\n", newThread);
        print_thread_id (newThread);

        // Push the message in its queue.
        serverInfo obj;
        obj.serverId  = serverNumber;
        obj.threadId = newThread;
        obj.queue.push_back (message);
        serverInfoVector.push_back (obj);

        condition++;
        pthread_cond_signal (&conditionVariable);
        pthread_mutex_unlock (&demoMutex);
    }
    else
    {
        // This server exists, so lookup its thread and queue, push the message in the corresponding queue.
        printf ("\nIn else ()`: thread id %ld\n", serverInfoVector [i].threadId);
        serverInfoVector [i].queue.push_back (message);

        condition++;
        pthread_cond_signal (&conditionVariable);
        pthread_mutex_unlock (&demoMutex);
    }
}

int main ()
{
    checkServerExists (1, "anisha");
    checkServerExists (2, "kaul");
    checkServerExists (1, "sanjeev");
    checkServerExists (2, "sharma");

    for (unsigned int i = 0; i < serverInfoVector.size (); i++)
        pthread_join (serverInfoVector [i].threadId, NULL);

    return 0;
}

Output:

In checkServerExists ()`: thread id 139875161245456
00007f37394c8710
In checkServerExists ()`: thread id 139875152852752
00007f3738cc7710
In else ()`: thread id 139875161245456

In else ()`: thread id 139875152852752

Segmentation fault
  • The problem is the seg fault which I am getting even when I have joined the threads in main.
  • On OpenSuse pthread_t is typedefed as unsigned long, so I ve tried to print it. Also, I have tried to call the function print_thread_id as shown here: https://stackoverflow.com/a/1759894/462608

Little improvement:

In checkServerExists ()`: thread id 139975945303824
00007f4eb07f3710
In checkServerExists ()`: thread id 139975936911120
00007f4eafff2710
In else ()`: thread id 139975945303824

In else ()`: thread id 139975936911120

Thread: 139975936911120, poped from queue: 1kaul1�&��N�&��Na7�Npq`�q`�q`'��N�s`�s`�s`!�q`!�q`�s`1sharma
                                                                                                       Segmentation fault

Now the poped value from second thread is getting printed, but still the seg fault persists.

Community
  • 1
  • 1
Aquarius_Girl
  • 21,790
  • 65
  • 230
  • 411
  • `std::vector` is not threadsafe. While (on first sight) it looks you only access `serverInfoVector` while holding the `demoMutex` from within `checkServerExists` and the thread function `printHello`, you don't while accessing it in your for-loop. This might not be the actual reason for the SIGSEGV, but it looks like a potential issue nevertheless. – Christian.K Mar 06 '12 at 06:43
  • @Christian.K Sorry, couldn't understand your point well. vector might not be thread safe but I have everytime put it in the locks. Why is it still a problem? – Aquarius_Girl Mar 06 '12 at 06:47
  • Yes, but you don't when you _read_ it in `main()`. Actually, looking closer it looks as if you don't actually modify `serverInfoVector` from within your threads, so it should be at least fixed in size and content of the `threadID` member when you come to execute the `for` loop. As I said it might not be the reason for the SIGSEGV, but it looks fragile anyway. YMMV. – Christian.K Mar 06 '12 at 06:55
  • @Christian.K `it looks as if you don't actually modify serverInfoVector from within your threads` Why do say that? In checkServerExists I do "insert" values in that vector. and in printHello I "remove" values from the vector. Secondly, while joining how I can use a mutex? Do I need it there? – Aquarius_Girl Mar 06 '12 at 07:14
  • `checkServerExists` still runs on the main thread, so all the insertions happen before the `for` gets executed. For all I can see, in `printHello` you don't remove or add elements from `serverInfoVector`, but to/from the `queue` member of each element. I'm starting to contradict myself here which might be confusing. What I'm saying is basically this: always access shared data while holding the same lock/mutex, even tough in your case it _looks_ that this is not the issue (see arguments above). You don't need the mutex for the join in itself. – Christian.K Mar 06 '12 at 07:17
  • 2
    Inside the `printHello` function, there is a loop that goes through the elements of the `serverInfoVector`. The condition on the loop is `i <= serverInfoVector.size()`. Note that it says `<=`, not `<`. Could be it runs beyond the end of the vector? – jogojapan Mar 06 '12 at 07:18
  • @Christian.K `checkServerExists still runs on the main thread` I realize that now. so, does this mean that no other thread will ever get the access to this function? Also, since I am creating the new thread in this function, and then inserting in the queue, do I still not need the locks? – Aquarius_Girl Mar 06 '12 at 07:30
  • The handling of the loop is still not quite correct. After the loop has finished, it may happen that `i == serverInfoVector.size()`. In the `if` statement that follows the loop, that can result in an attempt to access an invalid vector element. (Not sure though whether that explains the new error.) – jogojapan Mar 06 '12 at 07:30
  • @jogojapan Found the fault, see the answer. – Aquarius_Girl Mar 06 '12 at 07:39
  • @jogojapan `After the loop has finished, it may happen that i == serverInfoVector.size()` How can that happen? If size of vector is 5, i will not be greater than 4. – Aquarius_Girl Mar 06 '12 at 07:39
  • 1
    @Anisha Kaul When i is 4, it will be incremented for one last time. Only then will the loop terminate, and i will be 5. Put more generally, after a loop has terminated, the loop condition is usually _not_ valid any more. – jogojapan Mar 06 '12 at 07:43
  • @jogojapan I get your point, and I have written a senseless code :mad:. I have used `found = true` in that loop, but haven't used it as a condition checker: `while ((found == true) && (!serverInfoVector [i].queue.empty ()))` Now, I think, even if i gets to 5, it won't do any harm? would it? – Aquarius_Girl Mar 06 '12 at 07:47
  • 1
    @Anisha Kaul Yes, that sounds right. Time for a well-deserved break now, after all the debugging :) – jogojapan Mar 06 '12 at 07:52
  • @jogojapan Now since we are here, can you explain me some more "thread design" problems (if you wish to)? – Aquarius_Girl Mar 06 '12 at 07:54
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/8573/discussion-between-jogojapan-and-anisha-kaul) – jogojapan Mar 06 '12 at 08:13

1 Answers1

2

The segmentation fault was due to my NOT using pthread_equal for comparison of threadIds in the following code:

if (serverInfoVector.size () > 0)
    {
        while ((i < serverInfoVector.size ()) && (found == false))
        {
            if (*my_tid == serverInfoVector [i].threadId)
            {
                found = true;
                break;
            }
            else
                i++;
        }
    }

Thanks to @David Schwartz for mentioning it here: https://stackoverflow.com/a/9564998/462608 and :doh: to me for not listening carefully.

Community
  • 1
  • 1
Aquarius_Girl
  • 21,790
  • 65
  • 230
  • 411