Re: CSingleLock

David Schwartz wrote:
On Dec 26, 7:19 pm, b...@xxxxxxxxxxxxxx wrote:
Can someone verify that the unlocking does indeed take place after
the return statement in this code? (i.e. after the access of myvar
is complete)

MyClass Foo::getfoo(void)
{
CSingleLock mylock(&csection,TRUE);
return m_myvar;

}

For the record, I have always included a coding policy prohibiting
this type of code in every coding standard document I have written.
This is just bad, bad, bad.

It is fine to have a lock holder class. But it should *assert* if the
lock is not release in debug code and only release the lock in release
code.

This makes it practically impossible to use exceptions. To elaborate on that
point, it costs all benefits of exceptions, i.e. that they propagate up the
stack automatically without cluttering code with check-and-return
statements, because you have to wrap any code that might throw with a mutex
locked in try-catch-clauses that unlock the mutex and rethrow. Also,
concerning the OP's code, it requires copying returnvalues to temporaries
only to then unlock the mutex and return the returnvalue. If you also ban
use of exceptions that might be okay, but then you are not using C++ to its
full capacity (not even close) anyway, which is a shame IMHO.

The main problem is simply that it is too easy to think you hold the
same locks before the ending '{' as you did after the beginning '}'.

Whereas, if you see:

{
CSingleLock foo(...);
// some code
foo.Unlock();
// some more code
return;
}

You now know you must make a choice whether to add code before the
'foo.Unlock' or after. It is clear that at '}' you hold the same locks
as you did at '{'.

A majority of my code works with the critical section spanning a whole
function. Otherwise, you can still limit the scope (i.e. the CSingleLock
object does have an Unlock() method).

Lastly, but that is beyond this discussion, I tend to bundle access to an
object with holding a mutex that guarantees that the access is exclusive.
This is achieved using a smart pointer approach and gives code that is
typically correct (i.e. you can't forget to lock the object, although you
still face the danger of e.g. deadlocks) but still easy to maintain.


I'd be interested to know the bigger picture of your statement above.

cheers

Uli

.

Relevant Pages

  • Re: A scoped lock/unlock implementation in C++.
    ... mutex_locker wrapping the same mutex, more than one thread won't share ... But beyond that it will still work with thread local mutex_locker instances, since no two threads can have a nonzero lock count anyway. ... When entering a locked region, if the last entry is negative, you know ... In case of unlocks even the unlock count is insignificant. ...
    (comp.programming.threads)
  • Re: How to check if a mutex is still locked?
    ... Shared resource should only be handled inside critical regions ... particular mutex mentioned in the initial message. ... lock the_super_mutex ... unlock the_super_mutex ...
    (comp.programming.threads)
  • Re: CSingleLock - known behaviour?
    ... CSingleLock and all the MFC locking classes are complete crap, ... Now look at the unlock code: ... only do an unlock if it believes the lock is set, and the result of a successful unlock is ...
    (microsoft.public.vc.mfc)
  • Pthreads: how to insure mutex is unlocked when a thread dies.
    ... Anyway, I was planning to have this thread lock a mutex when it starts, ... unlock it when it's done, e.g., when the application notifies it to ... I'm concerned that if the thread dies before the application notifies ...
    (comp.programming.threads)
  • Re: A scoped lock/unlock implementation in C++.
    ... namely a guarantee that a scoped_unlock *will* unlock the mutex lock ... You may protect the mutex::lock and unlock functions and declare the ... lock function and a locked flag. ...
    (comp.programming.threads)
Loading