Leaving critical section after thread crash

Question

I've got thread executing commands from list

do
{
    commandExec->criticalSection.EnterCS();
    if (!commandExec->commands.empty())
    {
        commandExec->ExecuteCommand(commandExec->commands.front());
        commandExec->commands.pop_front();
    }
    else
        commandExec->criticalSection.SuspendThread();
    commandExec->criticalSection.LeaveCS();
} while (commandExec->maintainCommandExecution);

and second thread which adds commands to list:

criticalSection.EnterCS();
commands.push_back(Command(code, parameters));
criticalSection.LeaveCS();
criticalSection.ResumeThread();

First thread could crash while executing command, therefore critical section could not be accessed by second thread:

If a thread terminates while it has ownership of a critical section, the state of the critical section is undefined. Source

So, what's the good way to handle this problem? I could think of some solutions, but they seems tricky (adding third thread, second critical section, etc.)

(criticalSection it's just simple wrapper for CRITICAL_SECTION)

Answer 1

You can create a class LockCriticalSection, which lock a critical section in the constructor and unlock the critical section in the destructor.

Then, in your code you allocate a LockCriticalSection object where you want to start the lock. The critical section will be released automatically when the object LockCriticalSection goes out of scope (because the function terminates correctly or because of an exception)

The following is the code that takes care of locking and unlocking the critical section:

/// \brief This class locks a critical section in the
///         constructor and unlocks it in the destructor.
///
/// This helps to correctly release a critical section in
///  case of exceptions or premature exit from a function
///  that uses the critical section.
///
///////////////////////////////////////////////////////////
class LockCriticalSection
{
public:
        /// \brief Creates the object LockCriticalSection and
        ///         lock the specified CRITICAL_SECTION.
        ///
        /// @param pCriticalSection pointer to the CRITICAL_SECTION 
        ///                         to lock
        ///
        ///////////////////////////////////////////////////////////
        LockCriticalSection(CRITICAL_SECTION* pCriticalSection):
            m_pCriticalSection(pCriticalSection)
        {
            EnterCriticalSection(pCriticalSection);
        }

        /// \brief Destroy the object LockCriticalSection and
        ///         unlock the previously locked CRITICAL_SECTION.
        ///
        ///////////////////////////////////////////////////////////
        virtual ~LockCriticalSection()
        {
            LeaveCriticalSection(m_pCriticalSection);
        }
private:
        CRITICAL_SECTION* m_pCriticalSection;
};

And this is the modified source code from your question:

do
{
    {
        LockCriticalSection lock(&criticalSectionToLock);
        while (!commandExec->commands.empty())
        {
            commandExec->ExecuteCommand(commandExec->commands.front());
            commandExec->commands.pop_front();
        }
    } // here the critical section is released
    // suspend thread here
} while (commandExec->maintainCommandExecution);

Answer 2

You can use a Mutex instead of a critical section (but beware of the problem outlined in Understanding the consequences of WAIT_ABANDONED).