5
\$\begingroup\$

I was looking at the different possible threading methods that I could use in my single-threaded application. What I have written so far is a class that will lock the thread when I instantiate the object, then unlock the thread when the object becomes out of scope. Here is what the class looks like:

ProcessCritSec.h

#ifndef AFX_PROCESS_CRIT_SEC_H_
#define AFX_PROCESS_CRIT_SEC_H_

class CProcessCritSec 
{
    public:
        CProcessCritSec(CCriticalSection* pcCriticalSection);
        virtual ~CProcessCritSec();

    private:
        CCriticalSection*   m_pcProcessCritSect;
};

#endif // !defined(AFX_PROCESS_CRIT_SEC_H_)

ProcessCritSec.cpp

#include "StdAfx.h"
#include "ProcessCritSec.h"

CProcessCritSec::CProcessCritSec(CCriticalSection* pcCriticalSection)
{
    m_pcProcessCritSect = pcCriticalSection;

    if (m_pcProcessCritSect != NULL)
    {
        m_pcProcessCritSect->Lock();
    }
}

CProcessCritSec::~CProcessCritSec()
{
    if (m_pcProcessCritSect != NULL)
    {
        m_pcProcessCritSect->Unlock();
    }
}

When I want to lock a thread I call this in a function:

CProcessCritSec cThreadSafety(&m_cCriticalSection);

Is this a safe way to locking a thread? Or is there a safer way to making my application single-threaded? Also I have a side question: When I instantiate this object in a try/catch block like so:

try
{
    CProcessCritSec cThreadSafety(&m_cCritialSection);

    Foo();
}
catch(...)
{
    //Log Exception
}

Will the thread become unlocked if an exception occurred within Foo() and the catch block catches the exception?

\$\endgroup\$
5
\$\begingroup\$

Is this a safe way to locking a thread?

Yes, it's the recommended way: see for example std::lock_guard

However:

  • I don't see why the destructor is virtual
  • You should probably disable/prevent the copy constructor and assignment operator
  • The constructor should probably take a reference (which can't be null) not a pointer
  • Try to ensure (though you're probably doing this already) that the destructor will not throw an exception

Will the thread become unlocked if an exception occurred within Foo() and the catch block catches the exception?

It will unlock when CProcessCritSec is destroys, which happens as soon as it goes out of scope, which happens whenever you leave the try statement (before you enter any catch statement).

\$\endgroup\$
  • \$\begingroup\$ The virtual on the destructor was a coding standard that I followed in a job. So you're saying that if I have the constructor take a reference, I won't have to worry about checking if it's null and the application will return an error at compile time If i try to set it to null? \$\endgroup\$ – John Odom Feb 19 '14 at 16:17
  • 2
    \$\begingroup\$ If the parameter is a reference instead of a pointer, then: CProcessCritSec cThreadSafety(m_cCritialSection); is expected/legal; CProcessCritSec cThreadSafety(null); is a compiler error; and CCriticalSection* pcCriticalSection = 0; CProcessCritSec cThreadSafety(*pcCriticalSection); is a run-time error, but you are allowed to blame the person who wrote that statement (instead of blaming your CProcessCritSec class) for trying dereference a null pointer. I prefer a reference instead of a pointer, when I want to document that the value can't/mustn't be a null pointer. \$\endgroup\$ – ChrisW Feb 19 '14 at 16:25
  • \$\begingroup\$ Ok cool, this has answered my questions, Thanks! \$\endgroup\$ – John Odom Feb 19 '14 at 16:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.