0

I have a somewhat simple multithreaded application written using the C++ std::thread library for both Ubuntu 14.04 and Windows 8.1. The code is nearly completely identical except that I'm using the operating system respective libraries windows.h and unistd.h to use Sleep/sleep to pause execution for a time. They both actually begin to run and the Ubuntu version does keep running for a short time but then hangs. I am using the proper arguments to the sleep/Sleep functions since I know Windows Sleep takes milliseconds, while Unix sleep takes seconds.

I've run the code multiple times and on Ubuntu it never makes it past two minutes whereas I've run it on windows twice for 20 minutes and then multiple times for roughly five minutes each to see if I was just lucky. Is this just an incompatibility with the thread library or does sleep not do what I think it does, or something else? The infinite loops are there because this is a school project and is expected to run without deadlocks or crashing.

The gist is that this is a modified 4-way stop where cars who arrive first don't have to slow down and stop. We only had to let one car through the intersection at a time which takes 3 seconds to cross, hence Sleep(3000), and don't have to worry about turns. Three threads run the spawnCars function and there are four other threads that each monitor one of the four directions N, E, S, and W. I hope that it's understandable why I can't post the entire code in the chance some other student stumbles upon this. These two functions are the only place where code is different aside from the operating system dependent library inclusion at the top. Thanks.

edit: Since I've just gone and posted all the code for the project, if the problem does end up being a deadlock, may I request that you only say so, and not post an in depth solution? I'm new here so if that's against the spirit of SO then fire away and I'll try to figure it out without reading the details.

        /* function clearIntersection
        Makes a car go through the intersection.  The sleep comes before the removal from the queue
        because my understanding is that the wait condition simulates the go signal for drivers.
        It wouldn't make sense for the sensors to tell a car to go if the intersection isn't yet
        clear even if the lock here would prevent that.
        */
        void clearIntersection(int direction)
        {
            lock->lock();
            Sleep(3000);
            dequeue(direction);
            lock->unlock();
        }
        /* function atFront(int direction)
        Checks whether the car waiting at the intersection from a particular direction
        has permission to pass, meaning it is at the front of the list of ALL waiting cars.
        This is the waiting condition.
        */
        bool isAtFront(int direction)
        {
            lock->lock();
            bool isAtFront = cardinalDirections[direction].front() == list->front();
            lock->unlock();
            return isAtFront;
        }


        void waitInLine()
        {
            unique_lock<mutex> conditionLock(*lock);
            waitForTurn->wait(conditionLock);
            conditionLock.unlock();
        }
        //function broadcast(): Let all waiting threads know they can check whether or not their car can go.
        void broadcast()
        {
            waitForTurn->notify_all();
        }
    };

    /* function monitorDirection(intersectionQueue,int,int)
    Threads will run this function.  There are four threads that run this function
    in total, one for each of the cardinal directions.  The threads check to see
    if the car at the front of the intersectionQueue, which contains the arrival order
    of cars regardless of direction, is the car at the front of the queue for the
    direction the thread is assigned to monitor.  If not, it waits on a condition
    variable until it is the case. It then calls the function to clear the intersection.
    Broadcast is then used on the condition variable so all drivers will check if they
    are allowed to pass, which one will unless there are 0 waiting cars, waiting again if not the case.
    */
    void monitorDirection(intersectionQueue *intersection, int direction, int id)
    {
        while (true) //Do forever to see if crashes can occur.
        {
            //Do nothing if there are no cars coming from this direction.
            //Possibly add more condition_variables for each direction?
            if (!intersection->empty(direction))
            {
                while (!intersection->isAtFront(direction))
                    intersection->waitInLine();
                intersection->clearIntersection(direction);
                cout << "A car has gone " << numberToDirection(direction) << endl;
                //All cars at the intersection will check the signal to see if it's time to go so broadcast is used.
                intersection->broadcast();
            }
        }
    }
nav
  • 125
  • 11
  • 4
    *I'm using the operating system respective libraries windows.h and unistd.h to use Sleep/sleep to pause execution for a time.* Why? We have [`std::this_thread::sleep_for`](http://en.cppreference.com/w/cpp/thread/sleep_for) now. – NathanOliver Dec 11 '15 at 20:51
  • 2
    Possible deadlock? I don't think you've posted enough code for someone to help. – JJF Dec 11 '15 at 20:52
  • 1
    We would need an [mcve] to help you. – NathanOliver Dec 11 '15 at 20:55
  • off topic: [`random_device` is goofy on windows.](http://stackoverflow.com/questions/18880654/why-do-i-get-same-sequence-for-everyrun-with-stdrandom-device-with-mingw-gcc4) You may find you always get the same random numbers. – user4581301 Dec 11 '15 at 20:58
  • Hmmm. On second through that's only partly off topic. If you always get the same numbers and they happen to be numbers that don't expose your deadlock... – user4581301 Dec 11 '15 at 21:00
  • I suppose I will just have to post everything and remove it later, the code content anyway, if that's ok. – nav Dec 11 '15 at 21:20
  • Also thanks NathanOliver, I wasn't aware of that. – nav Dec 11 '15 at 21:50
  • That is a lot of code/text for one question on this forum. Would take me quite a while to dig through that lot. Have you narrowed it down a bit? – Ed Heal Dec 11 '15 at 21:51
  • Not yet, no, sorry. Still trying to consider where deadlocks could happen since the code doesn't throw exceptions or anything like that, it just stops. – nav Dec 11 '15 at 21:53

1 Answers1

2

Your culprit is likely your while (!isAtFront(...)) loop. If another thread gets scheduled between the check and the subsequent call to waitInLine(), the state of your queues could change, causing all of your consumer threads to end up waiting. At that point there's no thread to signal your condition_variable, so they will wait forever.

Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • Thanks, I've found a solution though I don't think it qualifies as good design. Since I have to hold the lock before waiting, I can check if I'm at the front once more before finally waiting without fear of the queue states being modified. If I am still not at the front, go ahead and wait, otherwise unlock and leave which means I do wastefully check the condition again to leave the loop but deadlock shouldn't happen. The other thing I was thinking of was a honk on car spawn but that would be a bit much, I think. – nav Dec 11 '15 at 23:45
  • Likely the _correct_ way to do this would involve the [`std::condition_variable::wait()`](http://en.cppreference.com/w/cpp/thread/condition_variable/wait) variant that takes a postcondition function as an argument. – Miles Budnek Dec 12 '15 at 02:48
  • 1
    Looks like I should be checking the documentation more often. I remember reading about that when I first took a look at condition_variable but figured my (wrong) way was ok at the time. – nav Dec 12 '15 at 03:28