7
\$\begingroup\$

Current best practice for using a lock_guard looks like this:

// introduce scope to take the lock
{
    lock_guard lock(sync);    // sync is an accessible mutex object
    do_protected_stuff();     // may throw, but the mutex is released regardless
}
// no exception, mutex is released in normal flow

I don't like the scope being introduce without a statement. I think it detracts from readability, and I think it's ugly. It occurred to me that what I wanted was the equivalent of modern Python's with statement:

with Lock() as lock:          # corresponds to lock+mutex in C++
    do_protected_stuff()

I thought about it for a while and designed a syntax hack that's gotten me partway there:

#include <iostream>
#include <boost/thread.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
using std::cout;
using std::endl;

//=============================================
// This is the "novel" part
// See main() for usage
//=============================================
// a utility class
class with_lock_
{
public:
    with_lock_(boost::mutex &mtx_) : lock(mtx_), new_lock(true) {}
    operator bool() { bool ret = new_lock; new_lock = false; return ret; }
private:
    boost::lock_guard<boost::mutex> lock;
    bool new_lock;
};
// a macro to hide the gory details
#define with_lock_guard(SYNC)   for (with_lock_ lk_(SYNC); lk_; )

//==============================================

// some global stuff for multi-thread action
// a mutex
boost::mutex sync;

// a value protected by the mutex
float zz = 0.0f;

// a thread function that will change the value and crow about it
void get_it_yourself()
{
    boost::lock_guard<boost::mutex> lock(sync);
    zz = 99999.99999f;
    cout << endl << "  ---  MINE!!" << endl;
}

//==============================================

int main()
{
    boost::thread runit;

    //=============================================
    // This is the intended use...
    // a little syntactic hack, inspired by Python
    //=============================================
    with_lock_guard (sync)
    {
        runit = boost::thread(get_it_yourself);
        cout << "Starting... ";
        boost::this_thread::sleep(boost::posix_time::millisec(500));
        cout << "OK!  Z = " << zz << endl;
    }

    // wait for that thread to complete
    runit.join();

    return 0;
}

Accept, for this discussion, that having a keyword to introduce the scope is preferable to introducing scope with just a brace. This hack lets you pretend you have a real with-like statement: you can follow it with a single or compound statement.

I do have a concern regarding the scope of the with_lock_ object. As I read things, at some level (persisting even into C++11?) the scope of a variable introduced within a for statement is the block enclosing the for. In practice, it's working OK for me with MSVS 2008; there is a compiler option regarding this, which defaults to the behavior I need in this situation.

Even without backing off from the basic idea, I see room for improvement. For one thing, a true with statement in C++ would be able to accept other types of RAII-based objects, as Python does. But so far in practice, I've only really the undesirable syntax with lock_guard.

What I'm looking for is a way to generalize this feature such that, at least, any mutex type, from any namespace, can be used. Better, if any lock type can be used. A cleaner implementation than abusing for or the operator bool() would be of interest.

Also: I'm not concerned if it breaks older compilers, but it would good to know if this works or doesn't in platforms/compilers other than Windows/MSVS2008+. Comments decrying the whole abomination are fine.

\$\endgroup\$
1
  • \$\begingroup\$ Microsoft has a paragraph at the bottom of their for loop page describing how the compiler determines the scope according to the flags it's given (default is the C++ standard: the variable is scoped to the loop body). This is for MSVC'03; the newer versions appear to have the same options available. \$\endgroup\$ Oct 8, 2012 at 19:35

1 Answer 1

4
\$\begingroup\$

Comments on Design

I like the idea in general but usually when I introduce a lock in a scope, the scope is usually the whole method so it does not look that ugly:

void MyClass::myMethod()
{
    lock_guard lock(sync);    // sync is an accessible mutex object
    do_protected_stuff();     // may throw, but the mutex is released regardless
}

But lets assume that you have situations that make your style worth while in using:
Now my problem is that you have situations where things are not as they seem:

with_lock_guard (sync)
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;

Only the first statement is inside the lock. Yet I get no compiler error (yes I know silly error but the idea is that you should design your class so that it is really hard to abuse even accidentally):

I would change the design slightly so that it used a lambda:

with_lock_guard(sync, []()
{
    runit = boost::thread(get_it_yourself);
    cout << "Starting... ";
    boost::this_thread::sleep(boost::posix_time::millisec(500));
    cout << "OK!  Z = " << zz << endl;
});

This way it will be harder to abuse accidentally.

Comments on code:

If you are going to have operator bool() conversion method then at least make it explicit so it can not accidentally be abused. But since you are calling this from your own macro I would have a method that you explicitly call. This makes the code easier to understand.

https://stackoverflow.com/questions/6242768/is-the-safe-bool-idiom-obsolete-in-c11

// Prefer to use explicit on bool to prevent accidental usage:
explict operator bool() { bool ret = new_lock; new_lock = false; return ret; }

But I would do this:

#define with_lock_guard(SYNC)   for (with_lock_ lk_(SYNC); lk_.MakeSureWeRunOnceOnly(); )
\$\endgroup\$
4
  • \$\begingroup\$ The issue of misusing the block also applies to a for or while or... I don't consider that to be a deficiency. The lambda approach is certainly reasonable, but MSVS2008 does not implement C++11. Due to weird political fu at work, it's not clear to me if C++03 codebases will be updated. \$\endgroup\$
    – Mike C
    Oct 8, 2012 at 22:32
  • \$\begingroup\$ I'm aware of the explicit option but I don't expect any users of the class except for that macro. For self-documenting clarity, I do like the MakeSure... method you've devised. \$\endgroup\$
    – Mike C
    Oct 8, 2012 at 22:34
  • 1
    \$\begingroup\$ @MikeC: The issue of a missing block is a really big problem. The reason is that you are trying to move people from the concept of the lock applying to everything after the lock to a concept where the lock only applies to the next statement. Look at the use case scenario: people that had working code before and switch naively from lock_guard lock(sync) to with_lock_guard (sync) now have broken code if there code is more than one statement long. The comparison with for, while is not justified as (they are only used as an implementation detail that should never be seen). \$\endgroup\$ Oct 9, 2012 at 16:16
  • \$\begingroup\$ I am a beliver that well written C++ can not be abused (if it can then the author has a design problem). Unfortunately this code is way to easy to use incorrectly. Also the fact the under normal situations is likely to be used incorrectly when updating existing code make it a real problem. \$\endgroup\$ Oct 9, 2012 at 16:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.