C++ Exception Handling In Critical Section (pthreads)

[Edit: (copied from a comment) As it turns out, the problem was elsewhere, but thank you all for your input.]

I have a shared container class which uses a s开发者_开发知识库ingle mutex to lock the push() and pop() functions, since I don't want to simultaneously modify the head and tail. Here's the code:

int Queue::push( WorkUnit unit )
{
    pthread_mutex_lock( &_writeMutex );
    int errorCode = 0;

    try
    {
        _queue.push_back( unit );
    }
    catch( std::bad_alloc )
    {
        errorCode = 1;
    }

    pthread_mutex_unlock( &_writeMutex );

    return errorCode;
}

When I run this in debug mode, everything is peachy. When I run in release mode, I get crashes at roughly the time when the driver program starts pushing and popping "simultaneously". Does the try/catch block immediately force an exit if it catches a std::bad_alloc exception? If so, should I enclose the remainder of the function in a finally block?

Also, is it possible that the slower debug mode only succeeds because my push() and pop() calls never actually occur at the same time?

In C++ we use Resource Acquisition Is Initialization (RAII) for guarding against exceptions.

Is this actually bombing after an exception? Far more likely from your snippet is that you just have bad synchronization in place. That starts with the name of your mutex: "writeMutex". This is not going to work if there is also a "readMutex". All reading, peeking and writing operations need to be locked by the same mutex.

Does the try/catch block immediately force an exit if it catches a std::bad_alloc exception?

No. If a std::bad_alloc is thrown inside the try {...} block, the code in the catch {...} block will fire.

If your program is actually crashing, then it seems like either your push_back call is throwing some exception other than bad_alloc (which isn't handled in your code), or the bad_alloc is being thrown somewhere outside the try {...} block.

By the way, are you sure you really want to use a try...catch block here?

plus

what does the pop look like

create a lock wrapper class that will automatically free the lock when it goes out of scope (as in RAII comment)

c++ does not have finally (thanks to mr stoustrop being stroppy)

i would catch std::exception or none at all (ducks head down for flame war). If u catch none then you need the wrapper

Regarding release/debug: Yes, you will often find race condition change between the two types of builds. When you deal with synchronization, your threads will run with different level of training. Well written threading will mostly run concurrently while poorly written threading the threads will in a highly synchronous manner relative to each other. All types of synchronization yield some level synchronous behavior. It as if synchronous and synchronization come from the same root word...

So yes, given the slightly different run-time performance between debug and release, those points where the threads synchronize can sometimes cause bad code to manifest in one type of build and not the other.

You need to use RAII
This basically means using the constructor/destructor to lock/unlock the resource.
This gurantees that the mutex will always be unlocked even when exceptions are around.

You should only be using one mutex for access to the list.
Even if you have a read only mutex that is used by a thread that only reads. That does not mean it is safe to read when another thread is updating the queue. The queue could be in some intermediate state caused by a thread calling push() while another thread is trying ti navigate an invlide intermediate state.

class Locker
{
    public:
        Locker(pthread_mutex_t &lock)
            :m_mutex(lock)
        {
            pthread_mutex_lock(&m_mutex);
        }
        ~Locker()
        {
            pthread_mutex_unlock(&m_mutex);
        }
    private:
        pthread_mutex_t&    m_mutex;
};

int Queue::push( WorkUnit unit )
{
    // building the object lock calls the constructor thus locking the mutex.
    Locker  lock(_writeMutex);
    int errorCode = 0;

    try
    {
        _queue.push_back( unit );
    }
    catch( std::bad_alloc )  // Other exceptions may happen here.
    {                        // You catch one that you handle locally via error codes. 
        errorCode = 1;       // That is fine. But there are other exceptions to think about.
    }

    return errorCode;
}  // lock destructor called here. Thus unlocking the mutex.

PS. I hate the use of leading underscore.
Though technically it is OK here (assuming member variables) it is so easy to mess up that I prefer not to pre pend '' to idnetifiers. See What are the rules about using an underscore in a C++ identifier? for a whole list of rules to do about '' in identifier names.

Previous code sample with Locker class has a major problem: What do you do when and if pthread_mutex_lock() fails? The answer is you must throw an exception at this point, from constructor, and it can be caught. Fine. However, According to c++ exception specs throwing an exception from a destructor is a no-no. HOW DO YOU HANDLE pthread_mutex_unlock FAILURES?

Running code under any instrumentation software serves no purpose whatsoever. You have to right code that works, not run it under valgrind.

In C it works perfectly fine:

pthread_cleanup_pop( 0 );
r = pthread_mutex_unlock( &mutex );
if ( r != 0 )
{
    /* Explicit error handling at point of occurance goes here. */
}

But because c++ is a software abortion there just no reasonable way to deal with threaded coded failures with any degree of certainty. Brain-dead ideas like wrapping pthread_mutex_t into a class that adds some sort of state variable is just that - brain dead. The following code just does not work:

Locker::~Locker()
{
    if ( pthread_mutex_unlock( &mutex ) != 0 )
    {
        failed = true; // Nonsense.
    }
}

And the reason for that is that after pthread_mutex_unlock() returns this thread very well may be sliced out of cpu - preempted. That means that the .failed public variable will be still false. Other threads looking at it will get wrong information - the state variable says no failures, meanwhile pthread_mutex_unlock() failed. Even if, by some stroke of luck, these two statements run in one go, this thread may be preempted before ~Locker() returns and other threads may modify the value of .failed. Bottom line these schemes do not work - there is no atomic test-and-set mechanism for application defined variables.

Some say, destructors should never have code that fails. Anything else is a bad design. Ok, fine. I am just curious to see what IS a good design to be 100% exception and thread safe in c++.