1

EDIT 1:

FILE            *fp;
pthread_mutex_t demoMutex;
unsigned short globalThreadIndex = 0;

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

std :: vector <serverInfo> serverInfoVector;


void * printHello (void* threadId)
{
    pthread_t *my_tid = (pthread_t *)&threadId;
    printf ("\nIn `printHello ()`: thread id %ld\n", pthread_self ());

    ***pthread_mutex_lock (&demoMutex);***

    unsigned int i = 0;
    char               found = false;

    if (serverInfoVector.size () > 0)
    {
        // The following code should be executed when and only when the vector isn't empty.
        ***pthread_cond_wait (&demoConditionVar, &demoMutex);***

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

        if (found == true)
        {
            pthread_t            writeToFile = pthread_self ();
            unsigned short iterate;
            for (iterate = 0; iterate < 10000; iterate++)
            {
                fprintf (fp, " %d %d",  iterate,         4);
                fprintf (fp, " %lu %lu", writeToFile, sizeof (pthread_t));

                if (!serverInfoVector [i].queue.empty ())
                {
                    fprintf (fp, " %c %u", 'A', 1);
                    fprintf (fp, " %lu %lu", serverInfoVector [i].queue.front (), sizeof (pthread_t));
                    serverInfoVector [i].queue.pop_back ();
                }
                fprintf (fp, "\n %lu %u", writeToFile, 1);
            }
        }
        ***pthread_mutex_unlock (&demoMutex);***
        }
    ***pthread_exit (NULL);***
}

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

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

    if (found == false)
    {
        pthread_t newThread [2];

        int           returnValue;
        if ((returnValue = pthread_create (&newThread [globalThreadIndex], NULL, printHello, (void*) &newThread [globalThreadIndex])) != 0)
        {
          printf ("\nerror: pthread_create failed with error number %d", returnValue);
        }
        printf ("\nIn checkServerExists ()`: thread id %ld\n", newThread [globalThreadIndex]);

        serverInfo obj;
        obj.serverId  = serverNumber;
        obj.threadId = newThread [globalThreadIndex];
        obj.queue.push_back (newThread [globalThreadIndex]);
        serverInfoVector.push_back (obj);

        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.
        ***pthread_mutex_lock (&demoMutex)***; 
        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.
        if (serverInfoVector.size () > 0)
            ***pthread_cond_signal (&demoConditionVar);***

        ***pthread_mutex_unlock(&demoMutex);*** 

        pthread_join (newThread [globalThreadIndex], NULL);
    }
    else
    {
    }
}


int main ()
{
    fp = fopen ("xyz", "w");
    ***pthread_mutex_init (&demoMutex, NULL);
    pthread_cond_t demoConditionVar = PTHREAD_COND_INITIALIZER;***

    checkServerExists (1, "anisha");
    globalThreadIndex++;
    checkServerExists (2, "anisha");

    return 0;
}

This code is improved, the problem is still there (the program hangs, second thread doesn't get shown up).

checkServerExists function (in the current case) causes a new thread to be created, and stored in the array newThread.

checkServerExists function launches a new thread,

When the thread gets created it will immediately call its function printHello and get blocked on the condition variable.

checkServerExists function then inputs the value in the global structure's queue, sets the signal for the thread to waken up.

Now, what's the point that I am missing?

EDIT 2:

FILE            *fp;
pthread_mutex_t demoMutex;
unsigned short globalThreadIndex = 0;

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

std :: vector <serverInfo> serverInfoVector;


void * printHello (void* threadId)
{
    pthread_t *my_tid = (pthread_t *)&threadId;
    printf ("\nIn `printHello ()`: thread id %ld\n", pthread_self ());

    ***pthread_mutex_lock (&demoMutex);***

    unsigned int i = 0;
    char               found = false;

    if (serverInfoVector.size () > 0)
    {
        // The following code should be executed when and only when the vector isn't empty.
        ***pthread_cond_wait (&demoConditionVar, &demoMutex);***

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

        if (found == true)
        {
            pthread_t            writeToFile = pthread_self ();
            unsigned short iterate;
            for (iterate = 0; iterate < 10000; iterate++)
            {
                fprintf (fp, " %d %d",  iterate,         4);
                fprintf (fp, " %lu %lu", writeToFile, sizeof (pthread_t));

                if (!serverInfoVector [i].queue.empty ())
                {
                    fprintf (fp, " %c %u", 'A', 1);
                    fprintf (fp, " %lu %lu", serverInfoVector [i].queue.front (), sizeof (pthread_t));
                    serverInfoVector [i].queue.pop_back ();
                }
                fprintf (fp, "\n %lu %u", writeToFile, 1);
            }
        }
        ***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].threadId)
            {
                found = true;
                break;
            }
            else
                i++;
        }
    }

    if (found == false)
    {
        pthread_t newThread [2];

        int           returnValue;
        if ((returnValue = pthread_create (&newThread [globalThreadIndex], NULL, printHello, (void*) &newThread [globalThreadIndex])) != 0)
        {
              printf ("\nerror: pthread_create failed with error number %d", returnValue);
        }
        printf ("\nIn checkServerExists ()`: thread id %ld\n", newThread [globalThreadIndex]);

        serverInfo obj;
        obj.serverId  = serverNumber;
        obj.threadId = newThread [globalThreadIndex];
        obj.queue.push_back (newThread [globalThreadIndex]);
        serverInfoVector.push_back (obj);

        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.

        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.
        if (serverInfoVector.size () > 0)
            ***pthread_cond_signal (&demoConditionVar);***


        pthread_join (newThread [globalThreadIndex], NULL);
    }
    else
    {
    }
        ***pthread_mutex_unlock(&demoMutex);*** 
}


int main ()
{
    fp = fopen ("xyz", "w");
    ***pthread_mutex_init (&demoMutex, NULL);
    pthread_cond_t demoConditionVar = PTHREAD_COND_INITIALIZER;***

    checkServerExists (1, "anisha");
    globalThreadIndex++;
    checkServerExists (2, "anisha");

    return 0;
}

In this edit, I have placed the lock on the top of checkServerExists function (this function deals with the global structure serverInfoVector)

Still hanging. :doh:

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Aquarius_Girl
  • 21,790
  • 65
  • 230
  • 411
  • Creating a self contained test case that we could use and test ourselves would be more useful than providing some code snippets. For example, what is `found` ? – PlasmaHH Feb 16 '12 at 09:28
  • @PlasmaHH sorry, I had deleted some code here, found was one of them: Let me present the whole code - it is not too big. – Aquarius_Girl Feb 16 '12 at 09:31
  • 1
    btw. I suggest that you enable warnings in your compiler and fix them. with -Wextra -Wall in gcc I get at least 7 warnings that I think you should care about, one of which might be the culprit (i not being initialized) – PlasmaHH Feb 16 '12 at 09:35
  • Here's a very simple rule: Never call `pthread_cond_wait` unless you have already arranged things such that another thread will signal the condition variable. If you're calling `pthread_cond_wait`, it should be to wait for some specific thing that you know another thread is going to do after which it will signal the condition variable. (Also, you should call `pthread_cond_wait` wait in a `while` loop. You may wake up but still need to wait.) – David Schwartz Feb 21 '12 at 08:50

2 Answers2

5

This code is broken.

Firstly, you are modifying variables inside checkServerExists without locking the mutex. This is undefined behaviour.

If you fix that, then you are also not signalling your condition variable outside the printHello function. Consequently, once the thread has blocked in the pthread_cond_wait call, it will only wake due to spurious wakes, and when another printHello thread signals it. You should call pthread_cond_signal at the point where you set the condition flag, rather than in printHello.

A condition variable is just a notification mechanism. You need to associate a predicate with it, which is the condition being waited for (in your case, condition!=0). You must ensure that the variables accessed when setting and testing the condition are protected by a mutex, and that this mutex is the one passed to pthread_cond_wait in order to avoid potential race conditions. When you set the variables to indicate that the sleeping thread should wake, then you call pthread_cond_signal.

I've modified your code slightly to make it work. In particular, I've put the loop round the pthread_cond_wait call, and unlocked the mutex before calling pthread_join so that the printHello thread can acquire the mutex and proceed. You should never hold a mutex lock across a thread join. This code could still be improved greatly --- amongst other things, it is not exception safe.

#include <pthread.h>
#include <stdio.h>
#include <vector>
#include <string>
FILE            *fp;
pthread_mutex_t demoMutex;
pthread_cond_t demoConditionVar;

unsigned short globalThreadIndex = 0;

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

std :: vector <serverInfo> serverInfoVector;

void * printHello (void* threadId)
{
    pthread_t *my_tid = (pthread_t *)threadId;
    printf ("\nIn `printHello ()`: thread id %ld\n", pthread_self ());

    pthread_mutex_lock (&demoMutex);

    unsigned int i = 0;
    bool found = false;

    while (serverInfoVector.empty())
        pthread_cond_wait (&demoConditionVar, &demoMutex);

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


    if (found)
    {
        pthread_t            writeToFile = pthread_self ();
        unsigned short iterate;
        for (iterate = 0; iterate < 10000; iterate++)
        {
            fprintf (fp, " %d %d",  iterate,         4);
            fprintf (fp, " %lu %lu", writeToFile, sizeof (pthread_t));

            if (!serverInfoVector [i].queue.empty ())
            {
                fprintf (fp, " %c %u", 'A', 1);
                fprintf (fp, " %lu %lu", serverInfoVector [i].queue.front (), sizeof (pthread_t));
                serverInfoVector [i].queue.pop_back ();
            }
            fprintf (fp, "\n %lu %u", writeToFile, 1);
        }
    }
    pthread_mutex_unlock (&demoMutex);
    pthread_exit (NULL);
}

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

    pthread_mutex_lock (&demoMutex);

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

    if (!found)
    {
        pthread_t newThread [2];

        int           returnValue;
        if ((returnValue = pthread_create (&newThread [globalThreadIndex], NULL, printHello, (void*) &newThread [globalThreadIndex])) != 0)
        {
            printf ("\nerror: pthread_create failed with error number %d", returnValue);
        }
        printf ("\nIn checkServerExists ()`: thread id %ld\n", newThread [globalThreadIndex]);

        serverInfo obj;
        obj.serverId  = serverNumber;
        obj.threadId = newThread [globalThreadIndex];
        obj.queue.push_back (newThread [globalThreadIndex]);
        serverInfoVector.push_back (obj);

        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.

        // Now, since something has been pushed in the vector, it makes sense to wake up the sleeping thread.
        pthread_cond_signal (&demoConditionVar);

        pthread_mutex_unlock(&demoMutex);

        pthread_join (newThread [globalThreadIndex], NULL);
    }
    else
    {
        pthread_mutex_unlock(&demoMutex);
    }
}


int main ()
{
    fp = fopen ("xyz", "w");
    pthread_mutex_init (&demoMutex, NULL);
    pthread_cond_init (&demoConditionVar, NULL);

    checkServerExists (1, "anisha");
    globalThreadIndex++;
    checkServerExists (2, "anisha");

    return 0;
}
Anthony Williams
  • 66,628
  • 14
  • 133
  • 155
  • I know, shameless spam, but in my answer, this is illustrated: http://stackoverflow.com/a/5538447/104774. I hope this helps. – stefaanv Feb 16 '12 at 12:27
  • Anthony, I have edited the code- it is better than the previous but still hangs. Please have a look. – Aquarius_Girl Feb 21 '12 at 07:46
  • Your second paragraph is incorrect. Look up man or docs on pthread_cond_signal. – Maxim Egorushkin Feb 21 '12 at 07:53
  • @MaximYegorushkin: `condition` was a variable in the original code, which Anisha has removed. – Anthony Williams Feb 21 '12 at 08:29
  • @AnishaKaul: You need to use `pthread_cond_wait` in a loop: `while(!this_thread_must_wake_up) pthread_cond_wait(&condvar,&mutex);` Otherwise you may be subject to spurious wakes, where the thread wakes without being signalled. – Anthony Williams Feb 21 '12 at 08:31
  • @AnishaKaul: You need to protect the `serverInfoVector` with the mutex, since it is accessed by both threads. Without that protection you have undefined behaviour. – Anthony Williams Feb 21 '12 at 08:34
  • Ah, the "loop" is necessary? I thought it is only about a condition. I'll try that. – Aquarius_Girl Feb 21 '12 at 08:34
  • Also, you mean - each and every place where I access serverInfoVector I need to put a lock? – Aquarius_Girl Feb 21 '12 at 08:38
  • @AnthonyWilliams: It's perfectly fine to signal a condition without holding the mutex. Whereas your 2nd paragraph says it's UB. – Maxim Egorushkin Feb 21 '12 at 08:44
  • Anthony, see the second edit now. please, and if it still doesn't make any sense, can you show the code yourself? – Aquarius_Girl Feb 21 '12 at 08:46
  • @MaximYegorushkin: I agree that it's fine to signal a condition variable without holding a mutex. The `condition` referred to in my 2nd paragraph was an integer or boolean variable in Anisha's original code. – Anthony Williams Feb 21 '12 at 09:12
  • 1
    See http://pastebin.com/Hvdvv7XU I've made various changes, but the code is still not clean. It doesn't hang any more though. – Anthony Williams Feb 21 '12 at 09:31
  • Anthony, thanks for the effort, - the major difference that I find in your and mine code is that I had written `pthread_mutex_unlock(&demoMutex);` AFTER `pthread_join`. Can you please post that code here in your answer, I'll select it. – Aquarius_Girl Feb 21 '12 at 09:49
0

I'm guessing that the second thread is not woken up from its wait. The first thread tries to signal the condition variable but the seconds hasn't even been started yet. I'm wondering why it even goes past the wait in the first thread, but it might just never wait because the thread starts executing after condition is already 1.

kossmoboleat
  • 1,841
  • 1
  • 19
  • 36