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)

You need what is called a scope-lock wrapping that critical section. it locks on scope entry (or manually), but is guaranteed to unlock (if it is locked) on scope exit, *including exit by exception*. — WhozCraig, Nov 07 '12 at 23:24
What do you mean the thread is "crashing"? I don't think a thread can "crash", only a process. — Mooing Duck, Nov 07 '12 at 23:54
You're right, it will crash down whole process, but single thread can be terminated by external process. This makes my question less meaningful though... — lowliet, Nov 08 '12 at 00:13

score 2 · Answer 1 · answered Nov 07 '12 at 23:46

2

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

answered Nov 07 '12 at 23:46

ChrisW

54,973
13
116
224

I also use SleepConditionVariableCS to suspend thread, so I would rather to stay this way. I'm currently trying scope-lock critical section as @WhozCraig suggested, but if this won't work out, I'll try Mutex instead. – lowliet Nov 07 '12 at 23:54

Paolo Brandoli · Accepted Answer · 2012-11-08T00:21:44.247

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);

[You have a destructor, but no copy-constructor nor assignment operator](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). Typically these are declared and not defined for mutex lockers (since copying such a thing doesn't make sense). Additionally, the virtual destructor is unnecessary. — GManNickG, Nov 08 '12 at 00:39
A critical section won't unlock, if the thread exits without releasing the critical section. — ChrisW, Nov 08 '12 at 19:40
@ChrisW if an exception happens in the loop then the critical section will unlock. Then, if the exception is not handled, the whole process will crash and the state of the critical section becomes irrelevant. — Paolo Brandoli, Nov 08 '12 at 20:37
Whether destructors are invoked depends on the type of exception: see http://msdn.microsoft.com/en-us/library/1deeycx5(v=vs.80).aspx Also a thread can exit `ExitThread` or be terminated `TerminateThread` without an exception, i.e. without unwinding the stack at all. — ChrisW, Nov 08 '12 at 21:03

Leaving critical section after thread crash

2 Answers2