LOCK CMutex, CSingleLock, CMultiLock, ?????

I have a multithreaded application, and I'm trying to set it up so that only one thread can pass through a paticular function at one time.

void FileWatcher::AddFileName(const CString& NewFile)
      m_lockingobj.Lock(); //If already lock, then waith until unlock

So what I bascially want is when the Lock member function is called, if another thread has it locked already, then the current thread should be on hold until the other thread unlocks it.

When I run the above code, I get an ASSERT error at runtime in the following MFC CSingleLock member function:

BOOL CSingleLock::Lock(DWORD dwTimeOut /* = INFINITE */)
      ASSERT(m_pObject != NULL || m_hObject != NULL);
      ASSERT(!m_bAcquired); // <--- <--- *** Error here ***

      m_bAcquired = m_pObject->Lock(dwTimeOut);
      return m_bAcquired;

Is CSingleLock the right class for the functionallity I'm looking for, should I be using something else.

This is pretty simple to do in UNIX with POSIX functions, but I can't seem to figure out the correct method using MFC.
LVL 30
Who is Participating?
drichardsConnect With a Mentor Commented:
>> Not sure what you mean by this.  Can you give more details, or different explanation?
This just means that if your thread locks an object n times, it must also unlock it n times in order to free the sync object for other threads.

It sounds like you want to use an event rather than a critical section or mutex.  Events are a little different in that you don't acquire them.  You just signal them in one hread while other threads are waiting for the signal.  In your case, you want the GeFileName thread to wait for something to be placed in a queue somewhere.  So you have GetFileName wait on the event and code somewhere else willset the event.

Events can be auto-reset or manual reset.  Auto reset events will release exactly one waiting thread and will remain set until a thread tries to wait on it.  A manula reset event, on the other hand, remains set until you expliciitly reset it.  Thus, the manual reset event will release ALL threads waiting on it until it is reset.  The code here uses the MFC CEvent.  If you don't plan on using CSingleLock, you could use the raw Windows event object.  In that case, m_WaitUntilInQue becomes a HANDLE and you use the CreateEvent, SetEvent(HANDLE hEvent), and WaitForSingleObject Win32 API functions (shown as comments in the example below).

    class Filewatcher
        CEvent m_WaitUntilInQue; // HANDLE m_WaitUntilInQueue;

    // Create an auto-reset event
    :m_WaitUntilInQue() // m_WaitUntilInQueue(CreateEvent(NULL, FALSE, FALSE, NULL))

    CString FileWatcher::GetFileName()
       m_WaitUntilInQue.Lock();// WaitForSingleObject(m_WaitUntilInQue, INFINITE);
       //Now get data in container and return it

Then, wherever your code is that puts something in the queue:

     m_WaitUntilInQue.SetEvent(); // SetEvent(m_WaitUntilInQue);

The other aspect of this is that the wait is infinite in GetFileName.  If you want to end the program, you need an additional signal to break the wait or time out the wait periodically and check for exit conditions.  You could also just terminate the waiting thread, but then your program must keep a handle to the thread in order to kill it later. One way to manage thread shutdown is to have a global manual reset event that is given to all synchronized classes via the constructor or an initialization method.  This code checks every second to see if it should exit.  You could just as easily use a simple flag rather than an event as the exit signal.  The advantage of the event is that in some cases you can save a bit of code by using WaitForMultipleObjects.

    CString FileWatcher::GetFileName()
        CSyncObject *so[] = { &m_WaitUntilInQue, &m_exitEvent }; // Don't need with Win32 version
        CMultiLock waitObjects(so, 2);  // HANDLE waitObjects[] = { m_WaitUntilInQue, m_exitEvent };
        if (waitObjects.Lock(INFINITE, FALSE) == WAIT_OBJECT_0) // if ( WaitForMultipleObjects(2, waitObjects, FALSE, INFINITE) == WAIT_OBJECT_0 )
            // Only get here if queue event is signaled.
            //Now get data in container and return it

Elsewhere the thread will have to examine the exit event to see if it should exit its thread function.

Depending on what the program is doing, it may be OK to just terminate the thread, but that's usually bad practice.
You can do that quite easily there, too. I I have to admit that I *never* used these MFC sync objects, since I always considered them superfluous. Whet you want can be achieved by using e.g.

class CLockingObj {


    CLockingObj () { InitializeCriticalSection(&m_cs);}
    ~CLockingObj () { DeleteCriticalSection(&m_cs);}

    void Lock() { EnterCriticalSection(&m_cs);}
    void Unock() { LeaveCriticalSection(&m_cs);}


AxterAuthor Commented:
I just tried it, but it doesn't seem to be working.

To test it out, I call Lock twice.

      m_WaitUntilInQue.Lock();//Code should never reach this point

When I debug the above code, it does reach the secon Lock.

FYI:  You have a typo in above code (Unock instead of Unlock)
Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

jkrConnect With a Mentor Commented:
>> I just tried it, but it doesn't seem to be working

That's not a bug, that's a feature:

"Once a thread has ownership of a critical section, it can make additional calls to EnterCriticalSection or TryEnterCriticalSection without blocking its execution. This prevents a thread from deadlocking itself while waiting for a critical section that it already owns." (from: http://msdn.microsoft.com/library/en-us/dllproc/base/entercriticalsection.asp)

>>FYI:  You have a typo in above code

Well, I shouldn't type code into a browser window :o)

jkr, you have reinvented the MFC CCriticalSection class with your CLockingObj.

As for CSingleLock, it is meant as a local wrapper for one of the basic sync object types.  It's only useful feature is that in it's destructor it releases the sync object so you don't have to have explicit calls to Unlock in your code.  This can be useful if your code throws exceptions not caught in the throwing function.

You use it like this, assuming m_lockingObj is a CCriticalSection (or maybe CMutex) member of the class:

    void FileWatcher::AddFileName(const CString& NewFile)
         CSingleLock lock(&m_lockingObj);
         lock.Lock();  //If already lock, then waith until unlock
         lock.Unlock();  // Optional with CSingleLock

You could just as easily use the CCriticalSection directly if you are not worried about abnormal execution - just change your m_lockingObj from CSingleLock to CCriticalSection.

As jkr notes, putting two calls to a critical section or mutex in succession will not cause a wait at the second one since a thread is allowed to acquire a sync bject multiple times.  This allows you to recurse safely.  Note, however, that the thread must do a parallel release of the sync object for each acquisition.
AxterAuthor Commented:
>>"Once a thread has ownership of a critical section, it can make additional calls to EnterCriticalSection or TryEnterCriticalSection without blocking its execution.
That explains a lot of the problems I've been having with my code.
But I'm not sure what's an easy work around, without having to completely change my code.

>> that the thread must do a parallel release of the sync object for each acquisition.

Not sure what you mean by this.  Can you give more details, or different explanation?

AxterAuthor Commented:
My main problem here is that I want a thread to create FileWatcher object, and then when it calls GetFileName member function, I want it to block until there's a valid filename to retrieve.

FileWatcher  filewatcher;

CString NewFile = filewatcher.GetFileName(); //Should block here

To accomplish this, I had a member function called m_WaitUntilInQue, which in the constructor I called the lock.

FileWatcher ::FileWatcher()

Then in GetFileName, I made another call to Lock.

CString FileWatcher::GetFileName()
   m_WaitUntilInQue.Lock();//This should block until another process Unlocks
   //Now get data in container and return it

Unfortunately, with this logic, both Locks are in the same thread.

I'm not sure what would be a good work around.

For now, I removed the lock code, and I have GetFileName do a check on the size of the container.  If the container is empty, I do a Sleep(1000), and then check again.
But I hate this method, since it takes more resources, and there's a one second delay when valid data arrives.
AxterAuthor Commented:
CEvent sounds like what I need.

I'll have to wait until Monday before I can test this out.

AxterAuthor Commented:
Thank you both very much.

There were two main problems with my code.
First problems, was that I didn't realize CSyncObjects could not lock itself on the same thread.
My second main problem, is that I was trying to use one class for all my multithreading needs.

I'm now using CEvent for the GetFileName and AddFileName.
And I'm also using CMutex to lock and unlock before modifying or using my m_ListFilesNeedProcessing container.

I'm also using CSemaphore to limit the number of threads created.

Now that I understand the difference between CSemaphore, CMutex, and CCriticalSection I feel much more confident in using them.
All Courses

From novice to tech pro — start learning today.