CSingleLock question

[tnarol]

Hi,

I'm using CSingleLock on a CCriticalSection object around the public methods of a class to ensure it is thread-safe.

I'd like to know if I can call a method inside another like this :

void Myclass::Method1()
{
CSingleLock singleLock(&m_CritSection);
singleLock.Lock();
...
singleLock.Unlock();
}

void Myclass::Method2()
{
CSingleLock singleLock(&m_CritSection);
singleLock.Lock();
...
Method1();
...
singleLock.Unlock();
}


I guess it won't work but I'm not sure ... If so I'd say I could leave and reenter the critical section like this :


void Myclass::Method2()
{
CSingleLock singleLock(&m_CritSection);
singleLock.Lock();
...
singleLock.Unlock();
Method1();
singleLock(&m_CritSection);
...
singleLock.Unlock();
}


Isn't it risky ?

[Andreas Masur]

The first version is fine...if the same thread tries to acquire a synchronization object, the access is granted automatically without blocking the execution. This prevents your thread from deadlocking itself while waiting for a synchronization object it already owns...

[Andreas Masur]

[ Redirected thread ]

[tnarol]

OK, so there is no problem acquiring a locked mutex inside the same thread... this makes me happy !

Thanks !

I have one more question about CSingleLock :

I'd like to encapsulate the operation of taking and releasing the mutex in methods of my class. Is it possible to create and use a static CSingleLock object of the class ? Or do I have to create a new CSingleLock object everytime I need it ?

[Arjay]

You should create a single lock instance each time you use it; otherwise you'll get some undefined behavior.

As an alternative to CSingleLock, check out the synchronization classes in the links in my signature line. Like CSingleLock, they are lightweight class wrappers that wrap sync primitives. However, unlike CSingleLock, they don't require an explicit unlock statement.

The classes offer wrappers for a critical section, mutex, and single writer multiple reader object and a 'autolock' class that locks the sync object to acquire the resource in its ctor and unlocks and releases the object in its dtor.

It works something like this:


// Declare the resource in the class declaration
CLockableCS m_csResource;

void SetString( LPCTSTR sz )
{
// Lock the resource
CAutoLockT< CLockableCS > lock( &m_csResource );
m_sResource = sz;
} // lock is automatically released here

[Andreas Masur]

I'd like to encapsulate the operation of taking and releasing the mutex in methods of my class. Is it possible to create and use a static CSingleLock object of the class ? Or do I have to create a new CSingleLock object everytime I need it ?
As being pointed out, static sychronization objects are usually rather a headache than a help.

As already indicated by Arjay, a common approach is the RAII (Resource Acquisition Is Initialization) idiom.

Using this, we first build a synchronization class that wraps the e.g. critical section object:

class lock_base
{
public:
lock_base() {}
virtual ~lock_base() {}
virtual void acquire(unsigned long timeout = 0xffffffff) = 0;
virtual void release() = 0;
};

class critical_section_sync : public lock_base
{
public:
critical_section_sync() { ::InitializeCriticalSection(&critical_section_); }
~critical_section_sync() { ::DeleteCriticalSection(&critical_section_); }
void acquire(unsigned long) { ::EnterCriticalSection(&critical_section_); }
void release() { ::LeaveCriticalSection(&critical_section_); }

private:
CRITICAL_SECTION critical_section_;
};

The above example also provides an abstract base class that allows to derive different synchronization objects later on. You will see the advantage in a minute.

In order to use this 'critical_section_sync' class as well as to provide an automated unlocking mechnism in any case, a guard class is provided:

class lock_guard
{
public:
lock_guard(lock_base& sync) : sync_(sync) { lock(); }
~lock_guard() { unlock(); }

private:
lock_base& sync_;

lock_guard(const lock_guard&);
lock_guard& operator=(const lock_guard&);
void lock() { sync_.acquire(); }
void unlock() { sync_.release(); }
};

This guard uses the natural features of C++ of constructing and destructing an object. In the constructor, the embedded synchronization object is automatically locked, in the destructor it is automatically unlocked. Since the destructor will always be called even in a case where an exception has been thrown, the embedded synchronizxation obkect will always be unlocked.

The second thing here, is that we use the feature of polymorphism here...as you can see the guard class takes a reference to a 'lock_base' type. Due to polymorphism we can however pass in any object that is derived from the abstract base class. In our case we use it to pass in the derived class 'critical_section_sync'.

struct shared_data
{
void add(type val)
{
lock_guard gate(sync_);
// add 'val' to 'array_'
}

const type& get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return it directly
}

private:
critical_section_sync sync_;
CArray<type, arg_type> array_;
};

If you decide that you need to share this data across process boundaries, you cannot use a critical section any longer. Due to the open architecture, the only thing you need to do is to derive a new synchronization class from your base.

class mutex_sync : public lock_base
{
public:
mutex_sync(bool initial_owner = false)
{
mutex_ = ::CreateMutex(0, initial_owner ? TRUE : FALSE, 0);
if(!mutex_)
throw std::exception("Could not create mutex");
}

~mutex_sync() { ::CloseHandle(mutex_); }

void acquire(unsigned long timeout = INFINITE)
{
switch(::WaitForSingleObject(mutex_, timeout))
{
case WAIT_FAILED:
throw std::exception("Could not wait for mutex");
case WAIT_TIMEOUT:
throw std::exception("Timeout occurred");
}
}

void release()
{
if(::ReleaseMutex(mutex_) == FALSE)
throw std::exception("Could not release mutex");
}

private:
HANDLE mutex_;
};

Then simply exchange the synchronization type used in your shared data...

struct shared_data
{
void add(type val)
{
lock_guard gate(sync_);
// add 'val' to 'array_'
}

const type& get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return it directly
}

private:
mutex_sync sync_;
CArray<type, arg_type> array_;
};

[MikeAThon]

OK, so there is no problem acquiring a locked mutex ....
Mutex or critical section? Your original code strongly implied that you were using a critical section. An MFC CCriticalSection works very well with CSingleLock, but IIRC, an MFC CMutex object is not really that well-suited for use with CSingleLock.

Andreas' post is fantastic; study it carefully.

Mike

[Andreas Masur]

An MFC CCriticalSection works very well with CSingleLock, but IIRC, an MFC CMutex object is not really that well-suited for use with CSingleLock.
Indeed, one should be careful using a 'CMutex' together with 'CSingleLock'. The problem is simply that it returns 'true' if the mutex is abandoned which can lead to fatal problems since data protection is no longer guaranteed.

[tnarol]

You should create a single lock instance each time you use it; otherwise you'll get some undefined behavior.

As an alternative to CSingleLock, check out the synchronization classes in the links in my signature line. Like CSingleLock, they are lightweight class wrappers that wrap sync primitives. However, unlike CSingleLock, they don't require an explicit unlock statement.


OK for your first remark.

For the second thing, I'm surprised you said CSingleLock requires explicit Unlock. Actually I've decided not to call unlock after reading this in MSDN :
"After use of the resource is complete, either call the Unlock function if the CSingleLock object is to be used again, or allow the CSingleLock object to be destroyed"

Most of time I need to lock things during a class method call and I consider the CSingleLock object is destroyed when leaving the method and therefore doesn't need to be unlocked...I hope this is not wrong !?

[tnarol]

Thanks Andreas for such a detailed answer. Your example is a bit upper my current knowledge so I think but I could figure out how to use it thanks to your example.

I'll use it to create a list class that would be thread safe.

The only problem I'm facing with this class now is to be able to iterate my list from outside the class. My first idea was to have a getFirst() and a getNext() method but this could fail if the list is updated by someone between 2 calls. I don't see any solution if I can't lock the class outside a method call... this is why I asked if it was possible to have the lock as a static member of the class. How would you do this ?

[Andreas Masur]

Most of time I need to lock things during a class method call and I consider the CSingleLock object is destroyed when leaving the method and therefore doesn't need to be unlocked...I hope this is not wrong !?
No, this is correct. The 'CSingleLock' is somewhat implemented following the RAII idiom that I mentioned earlier . However, it provides an additional way of using it for example as a class member where the scope is limited to the existance of the class. In order to use such a class without the help of the scope, one need to be able to explicitly lock and unlock the enclosed synchronization object. Thus, in my eyes it is simply throught halfway-through and thus should be avoided if possible. Nevertheless, this is my opinion.

[Andreas Masur]

The only problem I'm facing with this class now is to be able to iterate my list from outside the class. My first idea was to have a getFirst() and a getNext() method but this could fail if the list is updated by someone between 2 calls. I don't see any solution if I can't lock the class outside a method call... this is why I asked if it was possible to have the lock as a static member of the class. How would you do this ?
Well...first of all, I would ask why you need to implement your own list class since there alre already classes available such as the STL 'list' or the MFC 'CList' one. Nevertheless, with both of these classes, synchronization in a multithreaded environment needs to be provided from outside. These classes do not have a multithreaded protection for various reasons.

The problem with something like that is always that it is hard to maintain consistency in your locking and unlocking scheme otherwise you are open for deadlocks. Think about the simple problem you are trying to solve.

If you want to lock the list internally (at class level) while iterating over it, the simple part is of course the locking. You can lock the class if someone calls 'getFirst()'. However, when are you going to unlock the class? Of course, you could say...if I reach the end of my list (using 'getNext()').

This unfortunately would mean that everybody using the list class, really iterates through the whole list everytime. If someone is, however, looking for a value and only iterates over the list until he/she founds the value, you will never ever reach the end (unless the value isn't in the list orthe searched value happens to be the last one). Thus, you would never ever beable to unlock the class accordingly. This simply results in a list that isn't been usable after that.

You might consider separate functions such as 'Lock()' and 'Unlock()'. However, same problem here....how could you guarantee that the user is always calling 'Lock()' before accessing the list and 'Unlock()' after he/she is done.

In other words...adding synchronization at class level is pretty hard to obtain in a consistent, error-safe method so that you really should weigh the efforts against the usability for that...

[tnarol]

Thanks again for your answer Andreas. After messing around a bit with my list class I came to the same conclusions but I needed this to be confirmed. I think I'l eventually give up encapsulation and reduce my list class to a CList member and a CCriticalsection static member that users will have to manage themselves... mmm I don't like that !

[Arjay]

Most of time I need to lock things during a class method call and I consider the CSingleLock object is destroyed when leaving the method and therefore doesn't need to be unlocked...I hope this is not wrong !?My mistake, after using the AutoLock class that I mentioned earlier for years now, I forgot that CSingleLock unlocks when it goes out of scope.

In answer to your last question regarding breaking encapsulation. You can still wrap the your container class (CList, std::list, etc.) into a class that provides thread safety (hidden from users of the class). As far as iterating the container externally from the class, you'll need to either expose the lock/unlock functionality of the class to the externall callers, or you can make a copy of the data (by internally locking, copying, and unlocking) and return the copy.

[John E]

class lock_base
{
public:
lock_base() {}
virtual ~lock_base() {}
virtual void acquire(unsigned long timeout = 0xffffffff) = 0;
virtual void release() = 0;
};

class critical_section_sync : public lock_base
{
public:
critical_section_sync() { ::InitializeCriticalSection(&critical_section_); }
~critical_section_sync() { ::DeleteCriticalSection(&critical_section_); }
void acquire(unsigned long) { ::EnterCriticalSection(&critical_section_); }
void release() { ::LeaveCriticalSection(&critical_section_); }

private:
CRITICAL_SECTION critical_section_;
};

Hi Andreas - I came across your code which I'd like to use in a project but there are a couple of things I don't understand (highlighted by the red lines)

1) Is CRITICAL_SECTION a pre-defined type or do I need to #include something to get it?

2) How does 'timeout' work? I can't see where it gets used.

Thanks,

John.

[ Edit... ] Oops! I just realised that the 2nd question is silly. It's a requirement of the base class's pure virtual function but depending upon the derived class, it may or may not be required.... Still - where do I find the elusive CRITICAL_SECTION ?

[kirants]

CRITICAL_SECTION should be available when you include windows.h. It is a windows structure.

[John E]

Thanks for the help. I finally got some time today to play with this code. Here's the original sample of usage code from Andreas:-
struct shared_data
{
void add(type val)
{
lock_guard gate(sync_);
// add 'val' to 'array_'
}

const type& get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return it directly
}

private:
critical_section_sync sync_;
CArray<type, arg_type> array_;
};
Just to test the process, I modified the original code by adding a second call in the add() function, like so:-
struct shared_data
{
void add(type val)
{
lock_guard gate(sync_);
lock_guard gate2(sync_);
// add 'val' to 'array_'
}

const type& get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return it directly
}

private:
critical_section_sync sync_;
CArray<type, arg_type> array_;
};
Maybe I've misunderstood something but I assumed that the second declaration of lock_guard would never return (because the resource has just been locked by the first declaration). However, the 2nd declaration seems to return straight away - implying that the common resource was not locked. Have I misunderstood the concept?

[MikeAThon]

You might have a misunderstanding.

When dealing with synchronization objects, you need to focus on threads, not functions. The second call to lock_guard returns immediately because the thread that just acquired it is also able to acquire it a second time. It's only if another thread has the lock that the thread will not be able to acquire the lock.

Incidentally, there might be a bug in the get() function of Andreas' code:
const type& get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return it directly
}

The problem (as I see it) with this code is that it returns an unsynchronized reference to shared data. More specifically, when the function returns, the lock_guard variable goes out of scope, automatically releasing the synchronization object (this is correct and it's the way it's designed to operate). However, the get() function has returned a reference to the shared data. The calling function now has direct access to shared data, but the access is totally unsynchronized. It's true that the reference is a const reference, so the calling function can't write to it. But it can read from it, and it can read at the same time as when another thread is modifying it, or has even deleted it (if it was created on the heap).

Basically, the problem is that since the code returns a reference, the shared data is no longer protected. I think the get() function should return a copy, not a reference, so that the calling thread gets a copy only, unique to the thread. Then there is no longer an issue of unsynchronized access to shared data, since the dat is essentially thread-local.

const type get(int idx) const
{
lock_guard gate(sync_);
// get value of 'idx' from 'array_' and return a copy of it
return array_[ idx ];
}

I would leave the return type as a const, to reinforce the notion that the calling function is getting only a copy.

Andreas? Comments?

Mike

[MrViggy]

Nope. Once a thread locks a CS; it can make multiple "lock" calls which return immediately. See Andreas' second post:
if the same thread tries to acquire a synchronization object, the access is granted automatically without blocking the execution. This prevents your thread from deadlocking itself while waiting for a synchronization object it already owns...
Viggy

[John E]

Those explanations have cleared up my confusion. Thanks :wave:

[MrViggy]

NP! :D

Viggy

[John E]

Okay - dumb question coming up.... will a mutex also provide sync protection against multiple access by threads within the same process - or does it ONLY protect across processes?

In other words, why bother with a critical section, if a mutex can do both jobs?

[MikeAThon]

mutex (and others too) can be used to synchronize between threads of the same process.

The reason critical sections are preferred sometimes is that they are often faster in execution time. If a critical section is unowned when a thread tries to acquire it, it can become acquired in user mode, without the need for switching to kernel mode. A mutex, on the other hand, will always require a switch to kernel mode, even if it's unowned when a thread tries to acquire it.

Mike

[Arjay]

In other words, why bother with a critical section, if a mutex can do both jobs?Good question. Simple answer is that a critical section, being a user mode object, is faster than a mutex (which is a kernel object). Because the cs is a user object, unlike a mutex it can't be used across process boundaries nor can it be used within the WaitForXXX family of functions. The definitive book on the subject used to be Advanced Windows by Jeffrey Richter. His latest revision has been renamed "Programming Applications for Microsoft Windows (http://www.amazon.com/Programming-Applications-Microsoft-Windows-General/dp/1572319968)." It is an excellent book on processes, threads, synchronization and more.

Also, check out the articles listed in my signature line. They explain processes, threads, and synchronization and include source code for lightweight thread synchronization classes including an 'auto' synchronization class that locks on construction and releases on scope exit. The classes use the RIAA technique that Andreas mentioned and the articles are filled with several code examples that show inprocess critical section synchronization as well as synchronizing a memory mapped file shared across two processes using a mutex.