4 ms·

I would strongly disagree on RAII being a kludge in C++. The language feature that enables it is deterministic destructors, whose primary purpose is to ensure

I would strongly disagree on RAII being a kludge in C++. The language feature that enables it is deterministic destructors, whose primary purpose is to ensure that allocated resources are deallocated. RAII is one particular use of destructors. The primary goal is to have the object be the thing that owns a resource, not the calling scope. In terms of usability, destructors allow for resource management that is far easier at the call site than any other language. "with" blocks such as in python require the call site to be modified to include the explicit time of destruction, whereas C++ gets that for free from its existing scoping rules. "try/finally" such as in Java also requires modifying the call site, but at least has reasonable default behavior if accidentally omitted. "defer" feels like it has the worst of both worlds. It requires the call site to be modified when a resource is owned, and also requires the caller to know what the corresponding cleanup function is for any allocation.

dkersten 6y ago
I do feel that RAII is a little bit ruined by the fact that you cannot declare anonymous object instances. For example, I cannot write: lock (mutex); Expecting to declare an anonymous lock with the mutex passed in, as this gets parsed as a declaration of a lock called mutex (hopefully lock doesn’t have a default constrictor and I at least get an error). Instead I have to bake my lock: lock l(mutex); Often I have objects that only exist for RAII and it’s a little bit annoying to have to give them dummy names.
- cesarb 6y ago
  IIRC, the problem with C++ is that you can declare anonymous object instances... With surprising results (the object is destroyed at the semicolon, instead of being destroyed at the end of the block).
  - gpderetta 6y ago
    It is quite dangerous. IIRC some compilers do catch the issue and warn about it.
  - dkersten 6y ago
    Sure, you can, but its still not the behaviour you'd expect and the problem in my example is that "if it looks like a declaration, it is", so "foo bar;" and "foo (bar);" can be the same thing.
- gpderetta 6y ago
  it is a bit annoying yes. There are some proposals to have a anonymous reusable placeholders like '_', but they haven't gone anywhere yet. You can do lock(mutex), some-lock-protected-expression; but it is a bit too cute and limited. On the other hand, being able to name the RAII object is often very useful for example if you need to dismiss them early, which happens often in transactional code (unlocking a mutex before the end of scope is a relatively common occurrence). Also specifically for mutexes, a nice pattern is to bind object and mutex together to guarantee that the object is always used with the lock: sync<my_object> foo; { auto l = foo.lock(); // starts the critical section l->do_something() l->do_something_else(); } // it ends here // or if you only need to hold the mutex for a single call: foo->do_something(); // critical section lives only for the full expression
  - dkersten 6y ago
    > On the other hand, being able to name the RAII object is often very useful I didn't say it wasn't. Being able is perfectly fine, being forced is not. As I mentioned in another comment, the problem isn't the extra typing, the problem is that it requires us, the programmer, to remember to name it even when we don't feel we need and to remember that "foo (bar)" isn't calling the constructor of foo and passing in bar, its calling the default constructor and declaring bar. That's too many gotchas and rather error prone! I believe there was a cppcon talk where the speaker said that it was a very common bug in Facebook, despite that they have linter rules to catch this case. Its also a rather insidious bug, because the code will run seemingly normally, just... it never actually locks anything. A rather hard problem to debug too.
    - gpderetta 6y ago
      yes, I think I hit it at least once and flagged I don't know how many times in code reviews.
    - a1369209993 6y ago
      > Its also a rather insidious bug, because the code will run seemingly normally, just... it never actually locks anything. Actually, I think it's worse than that; IIUC, it will block until the mutex is unlocked, then run. Under light load, it will get away with this, but if the load is heavy enough, it will either guarantee that every process waiting for the lock runs at once, trampling each other's work, or almost-serialize them, making the race condition even more intermittent than if they didn't lock at all. Which failure mode you hit depends on how your scheduler works.
- jstimpfle 6y ago
  Workaround: #define LOCK(mutex) lock lock##__LINE__(mutex) Or even C compatible scoped locks: #define SCOPED_LOCK(mutex) \ for (int i##__LINE__ = lock_mutex(mutex), 1; \ i##__LINE__ --; \ unlock_mutex(mutex)) Use like SCOPED_LOCK(foo->mutex) { do_stuff(); }
  - dkersten 6y ago
    Of course there are workarounds, but that's not the point. First of all, your solution requires macros and the problems that come with that, but also, you now require programmers to remember to use your macro instead of just declaring the instance. The problem I'm describing is less about the effort of adding a variable name -- that's really not a big deal -- its that its super error prone to remember and these bugs slip through all the time. Requiring a variable name (either explicitly or through a macro like yours) is error prone.
- pjmlp 6y ago
  Sure you can, it just requires a little bit more syntax and following FP patters by making use of lambdas. With lock() being a function taking a lambda. lock([]{ // code locked with anonymous mutex });

dkersten 6y ago

I do feel that RAII is a little bit ruined by the fact that you cannot declare anonymous object instances. For example, I cannot write: lock (mutex); Expecting to declare an anonymous lock with the mutex passed in, as this gets parsed as a declaration of a lock called mutex (hopefully lock doesn’t have a default constrictor and I at least get an error). Instead I have to bake my lock: lock l(mutex); Often I have objects that only exist for RAII and it’s a little bit annoying to have to give them dummy names.

cesarb 6y ago
IIRC, the problem with C++ is that you can declare anonymous object instances... With surprising results (the object is destroyed at the semicolon, instead of being destroyed at the end of the block).
- gpderetta 6y ago
  It is quite dangerous. IIRC some compilers do catch the issue and warn about it.
- dkersten 6y ago
  Sure, you can, but its still not the behaviour you'd expect and the problem in my example is that "if it looks like a declaration, it is", so "foo bar;" and "foo (bar);" can be the same thing.
gpderetta 6y ago
it is a bit annoying yes. There are some proposals to have a anonymous reusable placeholders like '_', but they haven't gone anywhere yet. You can do lock(mutex), some-lock-protected-expression; but it is a bit too cute and limited. On the other hand, being able to name the RAII object is often very useful for example if you need to dismiss them early, which happens often in transactional code (unlocking a mutex before the end of scope is a relatively common occurrence). Also specifically for mutexes, a nice pattern is to bind object and mutex together to guarantee that the object is always used with the lock: sync<my_object> foo; { auto l = foo.lock(); // starts the critical section l->do_something() l->do_something_else(); } // it ends here // or if you only need to hold the mutex for a single call: foo->do_something(); // critical section lives only for the full expression
- dkersten 6y ago
  > On the other hand, being able to name the RAII object is often very useful I didn't say it wasn't. Being able is perfectly fine, being forced is not. As I mentioned in another comment, the problem isn't the extra typing, the problem is that it requires us, the programmer, to remember to name it even when we don't feel we need and to remember that "foo (bar)" isn't calling the constructor of foo and passing in bar, its calling the default constructor and declaring bar. That's too many gotchas and rather error prone! I believe there was a cppcon talk where the speaker said that it was a very common bug in Facebook, despite that they have linter rules to catch this case. Its also a rather insidious bug, because the code will run seemingly normally, just... it never actually locks anything. A rather hard problem to debug too.
  - gpderetta 6y ago
    yes, I think I hit it at least once and flagged I don't know how many times in code reviews.
  - a1369209993 6y ago
    > Its also a rather insidious bug, because the code will run seemingly normally, just... it never actually locks anything. Actually, I think it's worse than that; IIUC, it will block until the mutex is unlocked, then run. Under light load, it will get away with this, but if the load is heavy enough, it will either guarantee that every process waiting for the lock runs at once, trampling each other's work, or almost-serialize them, making the race condition even more intermittent than if they didn't lock at all. Which failure mode you hit depends on how your scheduler works.
jstimpfle 6y ago
Workaround: #define LOCK(mutex) lock lock##__LINE__(mutex) Or even C compatible scoped locks: #define SCOPED_LOCK(mutex) \ for (int i##__LINE__ = lock_mutex(mutex), 1; \ i##__LINE__ --; \ unlock_mutex(mutex)) Use like SCOPED_LOCK(foo->mutex) { do_stuff(); }
- dkersten 6y ago
  Of course there are workarounds, but that's not the point. First of all, your solution requires macros and the problems that come with that, but also, you now require programmers to remember to use your macro instead of just declaring the instance. The problem I'm describing is less about the effort of adding a variable name -- that's really not a big deal -- its that its super error prone to remember and these bugs slip through all the time. Requiring a variable name (either explicitly or through a macro like yours) is error prone.
pjmlp 6y ago
Sure you can, it just requires a little bit more syntax and following FP patters by making use of lambdas. With lock() being a function taking a lambda. lock([]{ // code locked with anonymous mutex });