code pattern for locking & unlocking

[Daniel Anderson]

Hi!

I often have to acquire locks and release them after I'm finished with
them.
I use constructor/destructor to acquire & release locks. everything
works fine.
sometime I must acquire a lock for few instructions, then release it
in the middle of a big function. I use braces to get timely
destructors, but I find the "middle of nowhere" braces kind of
embarrassing.
something like:

struct Lock
{
Lock(Mutex& mtx) mtx_(mtx) { mtx.lock(); }
~Lock() { mtx_.unlock(); }
operator bool() { return true;}
};

void someFunc()
{
// do some stuff
...
// now time to update share data
{
Lock myLock(data_mutex);
//use locked data
....
} // destructor called
// do more stuff
...
}

I would like to have something like the using keyword in c#.
Is there a way to fake it in C++ ?
for now I'm using an if to do it.


void someFunc()
{
// do some stuff
...
// now time to update share data
if (Lock myLock = Lock(data_mutex))
{
//use locked data
....
} // Unlock done by destructor

// do more stuff
...
}

Is it good programming ?
Is there a better way ?

Thanks

Daniel Anderson

 

[Gene Bushuyev]

Hi!

I often have to acquire locks and release them after I'm finished with
them.
I use constructor/destructor to acquire & release locks. everything
works fine.
sometime I must acquire a lock for few instructions, then release it
in the middle of a big function. I use braces to get timely
destructors, but I find the "middle of nowhere" braces kind of
embarrassing.

I guess it's a matter of taste. For me the curly braces are very
natural way of denoting the block.

something like:

struct Lock
{
Lock(Mutex& mtx) mtx_(mtx) { mtx.lock(); }
~Lock() { mtx_.unlock(); }
operator bool() { return true;}

};

Putting aside the errors in the code above, the only reason "operator
bool" exists in the class is to make if() construct possible, it's not
required for Lock proper functionality, which violates a good design
rule: make interface complete and minimal.

void someFunc()
{
// do some stuff
...
// now time to update share data
{
Lock myLock(data_mutex);
//use locked data
....
} // destructor called
// do more stuff
...

}

I would like to have something like the using keyword in c#.
Is there a way to fake it in C++ ?

That's one of the common mistakes people switching to another language
do -- is trying to fake what they are used to in a different language.
That usually causes grief for everybody who has to deal with that
code. Each language commands its own style, the only way to write good
code is to learn and use it.

for now I'm using an if to do it.

void someFunc()
{
// do some stuff
...
// now time to update share data
if (Lock myLock = Lock(data_mutex))
{

That assumes that Lock is copy-constructible or move-constructible,
which it's not -- because mutex is neither copy-constructible nor move-
constructible. Allowing that would lead to unsafe code causing
deadlocks or races.

 

[Matt Calabrese]

I would like to have something like the using keyword in c#.
Is there a way to fake it in C++ ?
for now I'm using an if to do it.

The whole point of C# "using" is to specify a scope for an object and
its disposal. It's a way to emulate the behavior that C++ already
supports naturally -- by simply writing object declarations in C++ you
get the behavior of C# using. I'm not sure why you are treating it as
though it's a special feature of C#.

Is it good programming ?
Is there a better way ?

Your "if" example is not "bad" programming, just sort of silly and
unnecessary (and likely more confusing to someone reading your code).
The "better" way IMO is to just use braces as in your original
example. What do you think is so bad about plain braces?

 

[Daniel Anderson]

The whole point of C# "using" is to specify a scope for an object and
its disposal. It's a way to emulate the behavior that C++ already
supports naturally -- by simply writing object declarations in C++ you
get the behavior of C# using. I'm not sure why you are treating it as
though it's a special feature of C#.


Your "if" example is not "bad" programming, just sort of silly and
unnecessary (and likely more confusing to someone reading your code).
The "better" way IMO is to just use braces as in your original
example. What do you think is so bad about plain braces?

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

So I'm trying to find a way that people will not remove my braces.

doing:

If (Lock Mylock(mutex)) {...}

could be read by others as: If I could lock this mutex, do this

while empty braces do not seem to convey more meaning than: hey I
starting a new block

I could have use a for loop, as it seems more natural to declare
variable
ex:

For(Lock myLock(mutex); myLock.IsLock(); MyLock.Unlock())
{
// do your deed
}

But the reading of the code does not convey the proper meaning (I
think)

Anyway, I understand people comments. I just came here for some
advices, and to see if someone thought about another way of doing the
locking/unlocking

 

[Daniel Anderson]

Since your Lock type depends on Mutex, why not just use the straight
Mutex and lock and unlock as you please?

Because of exception!

Some don't even know or haven long forgotten that you can create
blocks arbitrarily like this. I agree that it doesn't look nice, but
it has a lot of uses and i think there's nothing wrong with using it.
It's an incredibly simple and elegant way of solving this problem.

Simple, yes. I've doing it for a long time
elegant, I use to think so, until someone remove some braces in my
code that where there for RAII, since I do not think it is elegant

This is a C++ forum. C# is relevant to this question, but for those of
us who haven't worked with it, please post a short example.













You seem to be implying that this if statement will always evaluate to
true (since i assume this code is equivalent to your first example),
but when i see an "if" in any code, i always assume that it will
sometimes not evaluate to true. On the other hand, it reminds me of
this:
std::weak_ptr<int> wp;
void f() {
if( std::shared_ptr<int> sp = wp.lock() )
; // do something
}


What a subjective question!

according to Plato (or was it Socrates) good is not subjective

I think creating an arbitrary scope is one of the simplest and least
error prone ways to do this. I'm sure at least one person (if not you)
will disagree with me, but i like simple. I suppose you could create a

I do not disagree, I'm just looking for advice

class that inputs a function, locks, calls the function, then unlocks,
and you could use C++0x lambdas to integrate it into your current code
without too much refactoring, but that means you'll have to maintain
the locking class and every function/class/etc that uses it. I guess
i'm not the most articulate so i'll use someone else's words:
"Perfection is achieved not when you have nothing more to add, but
when you have nothing left to take away." ~ Antoine de Saint-Exupery.

this is a good guy this Antoine.

 

[Matt Calabrese]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

So I'm trying to find a way that people will not remove my braces.

I think you are trying to solve the wrong "problem" here. The issue
isn't clarity, it's a lack of competence of other programmers in the
language you are using. It would be one thing if you were doing
something relatively complicated such as template metaprogramming that
others may have trouble with, but you are simply using RAII for
scoping a lock, which is not exactly an uncommon odd idiom at all
whether it be for locks or for more general resource management. If
people are just randomly removing braces for no reason then they need
to be informed to simply not do that and understand why. Rather than
complicate code by introducing unnecessary control statements couldn't
someone just send out a brief email explaining that braces are not
superfluous because they are there for RAII? I realize that not
everyone has the same amount of experience with C++ and I understand
that it may seem subtle to people new to the language, but I can't
imagine that this would be something horribly difficult to teach. If
people understand what a destructor is then the point should be easy
to make clear.

Anyway, that's just my opinion on the matter. Obviously it's up to
your boss, but I would at least suggest this.

 

[Maxim Yegorushkin]

I often have to acquire locks and release them after I'm finished with
them.
I use constructor/destructor to acquire& release locks. everything
works fine.
sometime I must acquire a lock for few instructions, then release it
in the middle of a big function. I use braces to get timely
destructors, but I find the "middle of nowhere" braces kind of
embarrassing.
something like:

struct Lock
{
Lock(Mutex& mtx) mtx_(mtx) { mtx.lock(); }
~Lock() { mtx_.unlock(); }
operator bool() { return true;}
};

void someFunc()
{
// do some stuff
...
// now time to update share data
{
Lock myLock(data_mutex);
//use locked data
....
} // destructor called
// do more stuff
...
}

I would like to have something like the using keyword in c#.
Is there a way to fake it in C++ ?

The alternative to creating a scope just to unlock the mutex at a
particlular point, it may be simpler to provide unlock() function on the
lock, e.g.:

struct Lock
{
Mutex* mtx_;
Lock(Mutex& mtx) : mtx_(&mtx) { mtx_->lock(); }
~Lock() { this->unlock(); }
void unlock() {
if(mtx_) {
mtx_->unlock();
mtx_ = 0;
}
}
};

void someFunc()
{
Lock myLock(data_mutex);
// ... use locked data ...
myLock.unlock(); // unlock long before leaving the scope
// ... more code ...
}

Notice in the above how no artificial scope is introduced and
myLock.unlock() does exactly what it says.

for now I'm using an if to do it.


void someFunc()
{
// do some stuff
...
// now time to update share data
if (Lock myLock = Lock(data_mutex))
{
//use locked data
....
} // Unlock done by destructor

// do more stuff
...
}

Is it good programming ?

If statement naturally means it is conditional, so that a reader of such
code may be led to think that obtaining the lock may fail and the
control flow may skip the block guarded by if.

--
Max


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

 

[Francis Glassborow]

Simple, yes. I've doing it for a long time
elegant, I use to think so, until someone remove some braces in my
code that where there for RAII, since I do not think it is elegant

Well elegance is often a matter ot taste, but anyone who remove braces
from a piece of code without understanding the code gets much of what
they deserve. OTOH anyone who uses this kind of idiom without a comment
is being rather optimistic about the capabilities of maintenance
programmers.

Well if you really want something other than comments to stop idiots
breaking your code then

bool just_once(false);
do {
// place your code here
} while(just_once);

Though i think that is awful

according to Plato (or was it Socrates) good is not subjective

The word 'good' has several meanings and in the current context I yake
it to mean 'socially acceptable' (the society being programmers) as
opposed to morally good which I think was what the Greek philosophers
were meaning. And I guess that in classical Greek the words for the two
meanings were almost certainly different but it is more than 50 years
since I had to toil at classical Greek.


--
Note that robinton.demon.co.uk addresses are no longer valid.


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

 

[Paavo Helde]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

So I'm trying to find a way that people will not remove my braces.

In this case I would suggest
#define BEGIN_LOCKED_SECTION(mutex) {Lock myLock(mutex);
#define END_LOCKED_SECTION }

but I'm sure your boss has forbidden macros as well ;-)

 

[Hakusa]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

That sounds... less than ideal. Well, if it's really so bad, let me
expand on the solution i mentioned, but give an example this time.

template< typename Func >
void lock_do( Mutex& mtx, Func f )
{
Lock l( mtx );
f();
}

So now

void someFunc()
{
// do some stuff
...
// now time to update share data
{
Lock myLock(data_mutex);
//use locked data
....
} // destructor called
// do more stuff
...

}

becomes

void someFunc()
{
// do some stuff
...
// now time to update share data
lock_do( data_mutex, [](/*...*/) -> void {
//use locked data
....
} );
// do more stuff
...
}

I haven't tested the code because i have never used lambdas before in
actual code, but the idea's what's important. If you can't comment
your code, and you can't teach those around you about arbitrary
braces, but you can use c++0x features, this is a compromise. Without c
++0x, i guess you could still move all that code to external functions
if you wanted.

Of coarse, you always have the chance that someone comes by and
doesn't understand the code, but when they see that scary lambda
syntax, maybe they'll flee before modifying it.

according to Plato (or was it Socrates) good is not subjective

A little off topic, but:
We all know that charity is good and murder is bad, but think of how
many thousands of years of social evolution it took us humans to come
to that conclusion. In contrast, none of what we now call programming
existed even 100 years ago. We can't even all agree on what editor to
use, brace style, naming conventions, or language. One might say a
macro is objectively bad, but others might say when used right, macros
are objectively good.

Perhaps if good was not subjective, we wouldn't have to share our
ideas like this in order to be better at programming.

 

[Rick Wheeler]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

So I'm trying to find a way that people will not remove my braces.

The fact that someone removed your braces should be a testimonial to
your boss that comments are not in lieu of clarity, but are indeed
necessary to achieve it. Moreover, inventing a "using" substitute (and
even the if clause, for clause, or whatever) is an attempt to add
commentary through keyword expressions that somehow deserve to live.
In the end, they fall into the category of extraneous code which
contrarily obscures clarity. Unless you actually have an "else"
condition, the argument for the "if" becomes pure rationalization.
Most readers of C++ know exactly what you're doing when scoping code
blocks. And yes, most if not all of those authors would comment their
intentions if they were ever in doubt. Perhaps over time you can
respectfully enlighten your boss in these concepts.

I don't think your question about early unlocking was given much
discussion. But you can always add explicit lock/unlock members along
with sufficient state to avoid redundancy in those actions.

Rick

 

[Michael Doubez]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.

You can always name your lock variables:
{ Lock
Don_t_delete_this_brace_It_defines_the_lifetime_of_the_lock_of(mutex);
...
}

I suggest you put that as a snippet

Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

Time to be creative

assert( sedition && "this is not a comment" );

So I'm trying to find a way that people will not remove my braces.

Ok. So the solution may be to have a resetable lock:

struct Lock: non_copiable()
{
Lock(Mutex& mtx) mtx_(&mtx) { mtx.lock(); }
~Lock() { unlock(); }
void unlock() { if(mtx_) {mtx_->unlock();mtx_=0;} }

private:
Mutex* _mtx;
};

And use:
Lock lock(mutex)
....
lock.unlock();

You will have RAII unlock in case of early return and you have unlock
whenever you want.
The people after you will have to look for when the lock is released
but if it is what your boss does want.

 

[Goran]

The alternative to creating a scope just to unlock the mutex at a
particlular point, it may be simpler to provide unlock() function on the
lock, e.g.:

struct Lock
{
Mutex* mtx_;
Lock(Mutex& mtx) : mtx_(&mtx) { mtx_->lock(); }
~Lock() { this->unlock(); }
void unlock() {
if(mtx_) {
mtx_->unlock();
mtx_ = 0;
}
}

};

void someFunc()
{
Lock myLock(data_mutex);
// ... use locked data ...
myLock.unlock(); // unlock long before leaving the scope
// ... more code ...

}

Notice in the above how no artificial scope is introduced and
myLock.unlock() does exactly what it says.

I don't think that's good, not in the general case. It drastically
changes lock duration in face of exceptions. Without, lock is there
from ctor of lock to unlock(). With, it's from ctor to the end of the
scope. However smart we think we are, we will mess it up ;-).

Also, you introduced performance penalty (an if and an an assignment
to 0).

Goran.

 

[Miles Bader]

Rather than complicate code by introducing unnecessary control
statements couldn't someone just send out a brief email explaining
that braces are not superfluous because they are there for RAII? I
realize that not everyone has the same amount of experience with C++
and I understand that it may seem subtle to people new to the
language, but I can't imagine that this would be something horribly
difficult to teach. If people understand what a destructor is then the
point should be easy to make clear.

Or if this particular behavior is too pervasive to fix with an email,
put a comment in the code:

// This block delimits the scope of the GUARD variable
// (don't remove the braces!)
{
LockGuard guard (some_mutex);
...
}

Yeah, it's kinda ugly, but it serves to teach those people that were
incorrectly modifying the code why they shouldn't, instead of
obfuscating with "fake" syntax.

-miles

--
Any man who is a triangle, has thee right, when in Cartesian Space,
to have angles, which when summed, come to know more, nor no less,
than nine score degrees, should he so wish. [TEMPLE OV THEE LEMUR]


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

 

[James Kanze]

On Nov 27, 1:40 am, Matt Calabrese <[email protected]> wrote:

[...]

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

And the fact that people have removed your braces because they
didn't understand the code has proved that this last statement
doesn't hold.

My immediate reaction when reading your original posting was:
write shorter functions. Cases where you'd want to limit the
scope of a lock are rare. But realistically, they do occur.
And the only correct way of handling them is to add the extra
braces. Preferably with a comment. Ideally, all code should be
simple enough to be understood without comments, but
practically, code solving a complicated problem will require
some complexity somewhere.

So I'm trying to find a way that people will not remove my braces.

There are two parts to the correct solution. The first is to
get rid of anyone who deletes code without understanding why it
was there to begin with. And the second is to add a comment so
the reader understands without even having to analyse the code
why they are there. If your boss disagrees with either of these
solutions, the case is hopeless.

 

[Francis Glassborow]

I would like to have something like the using keyword in c#.
Is there a way to fake it in C++ ?
for now I'm using an if to do it.

The whole point of C# "using" is to specify a scope for an object and
its disposal. It's a way to emulate the behavior that C++ already
[...]
The "better" way IMO is to just use braces as in your original
example. What do you think is so bad about plain braces?

I've worked with plenty of people at different places.
where I work now, we have plenty of "not talented" programmer (maybe
I'm one of them) . In the past it happen that some of my stand alone
braces where removed by some programmers that did not understand the
idioms.
Now I cannot put comments to say why I've put braces, as my boss
forbid them.
For him putting comments means that the code is not clear and he does
not want code that is not clear.
Also he reasoned that if there is no comments, people will have to
understand the code before modifying it.

So I'm trying to find a way that people will not remove my braces.

Oh my

In this case, I would reccomend a macro. Seriously. It may actually be
the least trouble.

#define START_LOCKSCOPE(mtx) \
{ Lock localLock(mtx); \
/**/

void someFunc()
{
// do some stuff
...
// now time to update share data
START_LOCKSCOPE(data_mutex)
Lock myLock(data_mutex);
//use locked data
....
}
// do more stuff
...
}

Of course, better would be to:
* Educate your colleagues about this idom. (It's not *that* hard)
* Convince your boss that some comments might be ok.

And now you have a different problem, getting your editor to understand
the mismatched braces

If only managers would understand that the general guideline to minimise
comments is exactly so that those that are left stand out. Anytime there
is no reasonable way to express your intent clearly in code that can be
understood by idiots, is time for an all uppercase comment along the
lines of:

// IF YOU DO NOT UNDERSTAND MY CODE LEAVE IT ALONE

 

[Daniel James]

Well if you really want something other than comments to stop idiots
breaking your code then

bool just_once(false);
do {
// place your code here
} while(just_once);

Though i think that is awful

Perhaps the message is better conveyed by:

const bool object_lifetimes_matter(true);

if( object_lifetimes_matter )
{
// place code here
}

Still fairly awful, but at least it offers some education to code
reviewers to who might otherwise be tempted to break the code by
removing the braces!

It's self-commenting, and won't impose any runtime overhead as the
compiler can easily optimize it away ... the biggest problem is that
some fool may decide that object lifetimes don't matter and change the
initialization of the bool, effectively removing the whole code block
from the program.

Cheers,
Daniel.

 

[Öö Tiib]

If only managers would understand that the general guideline to minimise
comments is exactly so that those that are left stand out. Anytime there
is no reasonable way to express your intent clearly in code that can be
understood by idiots, is time for an all uppercase comment along the
lines of:

// IF YOU DO NOT UNDERSTAND MY CODE LEAVE IT ALONE

That is too radical. There are different levels of understanding.
For example some usual piece of code. Most readers understand what it
does if naming is not cryptic or misleading. On most cases the
misunderstanding is because reader does not understand why it does
that in the way it does.

For example "why updating shared data is unconditional?", "why the
scoped lock needs to unlock in middle of function not at end?" or "why
the scope that unconditionally updates shared data is not useful as
separate function?". At least something of it might be perhaps hard to
understand from OP's example.

 

[Maxim Yegorushkin]

I don't think that's good, not in the general case. It drastically
changes lock duration in face of exceptions.

Lock duration is the same since the destuctor still does unlock. The
difference is that there is no need for an extra scope when unlock() is
used.

> Without, lock is there
from ctor of lock to unlock(). With, it's from ctor to the end of the
scope.
And?

> However smart we think we are, we will mess it up ;-).

I don't understand you concern. Could you elaborate please?

Also, you introduced performance penalty (an if and an an assignment
to 0).

Quantify the impact please.

If that code is inline it is going to be optimized out. gcc, for
example, does this optimization even in debug mode.
http://en.wikipedia.org/wiki/Static_single_assignment_form