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++:
OP here. Totally agree. TIL about RAII.
That particular codebase doesn't use c++ exceptions, but still not a reason for a roll-your-own solution if something as efficient is in the standard library.
Ok, should've been more specific -- it doesn't catch any exceptions and lets them bring down the program and have the project maintainer check what they did wrong.
This. For at least three typical 'best practice' reasons. I know there are times best practices can be nicely ignored, but this is not one of those occasions. On the contrary. Let me lay them out again:
- these are the days of C++11 where scoped locks are readily available
Which is mostly due to lack of education. If you don't ever spent time learning, but just coding, you don't know better. And sometimes it's all too easy to get thrapped into it due to deadlines and whatnot.
A former collegue of mine, unfortunately, was the best example: programmed for like 15 years years and still wrote like it was all 'C with classes' (not that there ever was such a thing). You know, writing declarations at the top of functions etc. Missing out on C++11 is just as bad.
The Google C++ style guide has its quirks, but describing it as "C with classes" is flat out wrong.
> no boost
Not true, select parts of Boost are available.
> no lambda
The C++11 features are being rolled in gradually. Lambda expressions are now permitted.
> no rtti
The guide says "avoid RTTI" and urges the developer to come up with a different design. This is very much in line with C++ best practice overall. There's no ban on RTTI.
Yeah, they even admit that their macro compiles to identical code as the original RAII version. Where's the "overhead"?
As a general rule, if your C++ compiler can statically see the layout of your structures and the implementation of your functions, it will produce code as efficient as inlining everything yourself. Sufficiently complex functions make the benefits of inlining a judgment call on the compiler's part, but for something as simple as an RAII scoped lock, the abstraction is free.
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).
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
Exception issues aside, an odd definition of "scoped". A very non-C++ solution. Use RAII and lock_guard (if possible) as vinkelhake demonstrates. Consider,
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.
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.
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)
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.
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.
longjmp is what the library and standards writers refer to as a "non-local goto" so it's kind of to be expected that it would manage to bypass destructors (I'm not sure how you'd unwind the stack enough to ensure that you've destructed everything created since the corresponding setjmp).
It's not terribly surprising, but it is worth noting since one use of setjmp/longjmp is to implement exceptions, and one would expect (and desire) destructors to run when unwinding on exceptions.
> 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++:
GCC 4.8.1 generates identical code for process1 and process2.