
In the C++ bag of tricks: scoped locks - s_kanev
http://blog.skanev.org/2014/02/in-c-bag-of-tricks-scoped-locks.html
======
vinkelhake
The real advantage of a scoped lock is that it promises to release the mutex,
no matter how the scope is exited. If this LOCKED macro is used and an
exception is thrown from within the block, that mutex now stays locked.

> The only gotcha is, there is some overhead involved in this approach. The
> class instance takes up some space on the stack (several bytes) for every
> lock acquisition.

Not so fast. A decent compiler will eliminate the overhead and not actually
allocate stack space for the lock. Here's an example using one of the lock
guards in C++:

    
    
        struct foo {
          int var;
          std::mutex m;
        };
        
        void process1(foo& f) {
          std::lock_guard<std::mutex> lock(f.m);
          ++f.var;  
        }
    
        void process2(foo& f) {
          f.m.lock();
          ++f.var;  
          f.m.unlock();
        }
    

GCC 4.8.1 generates identical code for process1 and process2.

~~~
infogulch
Wait... if it's identical, how can it be safe against exceptions? `process2`
isn't.

~~~
saurik
In addition to twoodfin's answer, I will point out that even if the code could
throw, the code for the function would still be identical for when a throw did
not occur (as in, we would see identical output with the addition of some code
that would seemingly never be used or executed), on "good architectures", due
to zero-cost exceptions (which only incur overhead during unwind; if no
exception is thrown the code executed is exactly identical to if exceptions
were not possible).

------
fighterjet
I fail to see the point of using a macro (ew) to accomplish something that's
included, with several different implementations, in the c++11 standard
library

------
taway2012
Don't do this. The whole thing gave off a bad smell on first reading so I
would never have done this.

Thank you vinkelhake for nailing the the real problem (exceptions) and
bothering to check whether the stack overhead really exists.

Just wanted to chime in with a comment because HN doesn't show vote counts, so
future readers will "see" my upvote on vinkelhake's comment.

------
jacobparker
Exception issues aside, an odd definition of "scoped". A very non-C++
solution. Use RAII and lock_guard (if possible) as vinkelhake demonstrates.
Consider,

    
    
        LOCKED(lk_var) {
            return 42; // whoops
        }

------
VikingCoder
You're also consuming the variable name i, which everyone else is probably
using.

Speaking of which, this means you can't have one of your locks scoped inside
another one.

~~~
cjensen
Solvable by creating a Preprocessor macro which creates a unique and complex-
named identifier.

    
    
      #define LOCKED(lk) for(int UNIQUE_ID=0; UNIQUE_ID<1 && !lk_lock(lk); lk_unlock(lk), UNIQUE_ID++)
    

Use __LINE__ to help make the id unique.

~~~
VikingCoder
Then you still can't forget and do this:

    
    
        lock(lk_var) lock(lk_var2)
        {
            lk_var = lk_var2;
        }
    

Or this:

    
    
        lock(lk_var) { lock(lk_var2 { lk_var = lk_var2; } }
    

Using the pre-processor to solve a regular, every day, hum-drum, solved-a-
billion-times RAII problem is bad news.

------
steveklabnik
This kind of thing is one area I'm excited to see work in Rust: you can use
Rust's safety guarantees to ensure that all access goes through the lock.

------
JoeAltmaier
Works, but requires {} for scope. The local-object version can be scoped
however you like. It optimizes away just fine, similar to the for loop.

------
ef47d35620c1
C++11 has scoped locks built-in, doesn't it?

~~~
steve-howard
It offers a variety of them, simplest of which is lock_guard:

[http://en.cppreference.com/w/cpp/thread/lock_guard](http://en.cppreference.com/w/cpp/thread/lock_guard)

------
infogulch
What about:

    
    
         #define LOCKED(lk) for(int i=lk_lock(lk); i == 0; lk_unlock(lk), i++)
    

Probably better to test for 0 exactly, in case it fails. You don't want it to
unlock twice just because it returns -1.

~~~
eps
This is so wrong on so many levels.

~~~
infogulch
It's not any worse than what was suggested in the linked article...

------
8_hours_ago
A very important gotcha with this approach is that any variable named "i" will
be reset to 0 inside of the lock, and modifying "i" inside of the lock can
cause an infinite loop.

This can be mitigated by generating a unique variable name with the C
preprocessor: [http://stackoverflow.com/questions/1132751/how-can-i-
generat...](http://stackoverflow.com/questions/1132751/how-can-i-generate-
unique-values-in-the-c-preprocessor)

That being said, it's much better to use a standard method of scoped locking
instead of trying to reinvent the wheel.

------
ambrop7
I learned about the for loop hack from avr-libc[1][2]. It even does some
preprocessor abuse to implement different kinds of atomic and non-atomic
sections.

[1] [http://www.nongnu.org/avr-libc/user-
manual/group__util__atom...](http://www.nongnu.org/avr-libc/user-
manual/group__util__atomic.html)

[2] [http://svn.savannah.nongnu.org/viewvc/trunk/avr-
libc/include...](http://svn.savannah.nongnu.org/viewvc/trunk/avr-
libc/include/util/atomic.h?root=avr-libc&view=markup)

------
mojuba
Wouldn't this be exception-safe, provided lock_iter defines a ctor, dtor,
operator bool() and operator ++()?

    
    
        #define LOCKED(lk) for (lock_iter i = lk; i; i++)
    

(Edit: I thought lock_iter could be an empty class but no, it can not, so I
edited my post. lock_iter will have an internal flag that should be optimized
away following the same logic int i is optimized in the OP's version)

------
ska
People have already pointed out the specific problems with this approach so I
won't repeat them.

It's worth reiterating, though: If you are writing c++ these days and find
yourself reaching for a preprocessor macro, I think that's a good sign you
should think really carefully about what you are actually trying to do.
Chances are really good there is a (much) better way.

------
dllthomas
You can do some of this with the cleanup attribute in GCC, although it doesn't
trigger in the case of a longjmp.

~~~
khc
if you can rely on gcc, cleanup attribute is probably the best you can do. The
solution in original post doesn't cope with "break", "continue", "return" (and
of course, "goto", but I don't remember if cleanup attribute handles that
either).

Looks like their project already uses C++ though, so why don't they just use
C++ for this is beyond me.

~~~
dllthomas
Agreed.

------
cbsmith
Boost.Scopexit addresses this point pretty well I think:
[http://www.boost.org/doc/libs/1_55_0/libs/scope_exit/doc/htm...](http://www.boost.org/doc/libs/1_55_0/libs/scope_exit/doc/html/index.html)

------
zeroonetwothree
folly::Synchronized implements this in a correct way (and also prevents access
without acquiring the lock):

SYNCHRONIZED(data) { // modify data }

[https://github.com/facebook/folly/blob/master/folly/Synchron...](https://github.com/facebook/folly/blob/master/folly/Synchronized.h)

------
blt
This is not exception-safe like scoped locks are.

