2

I'm working with a C++ multithreaded program that has to read and write some attributes within a class using boost shared_mutex. The program creates multiple object instances of a class A while it is executing. This class A has an static shared_mutex that is shared by all class A objects. If one of the attributes of that class needs to be written in one of the object instances, the particular object instance needs to get exclusive access to the shared mutex by upgrading it to a unique lock. However, before doing that the set method checks if the class instance owns the lock, which looks a bit odd to me.

This is more or less how class A hpp file looks like:

#include <boost/thread.hpp>

class A{
    private:
        //Private methods
        void setAttribute(boost::upgrade_lock<boost::shared_mutex>& lock, const bool value);
        //Private variables
        static boost::shared_mutex mainMutex;
        mutable bool attribute;
    public:
        //... Some public methods using setAttribute
}

And this is how class A cpp file looks like:

void A::setAttribute(boost::upgrade_lock<boost::shared_mutex>& lock, const bool value)
{
    (void)lock;
    assert(lock.owns_lock());

    //get exclusive access to the lock
    const boost::upgrade_to_unique_lock<boost::shared_mutex> ulock(lock);
    attribute = value;
}

So, my question is why would I need to check if the lock passed owns the lock given that the method setAttribute is private and it's only used by methods within the class. Are there any circumstances where I should consider doing this?

Laura Rozo
  • 33
  • 4
  • 1
    `(void)lock;` what is that? –  Jun 14 '17 at 21:19
  • From the examples I've seen about how to use shared_mutex, they always invoke lock() and then upgrade to a unique lock to get exclusive access. Thus, '(void) lock' appears to call an overloaded operator that invokes the lock() function for the upgrade_lock? However, I'm not a 100% sure of this since I haven't been able to confirm it with the research I've done so far – Laura Rozo Jun 14 '17 at 22:45
  • "From the examples I've seen " - link? –  Jun 14 '17 at 22:56
  • This is one of the examples I saw about how to use shared_mutex: [link](https://stackoverflow.com/questions/989795/example-for-boost-shared-mutex-multiple-reads-one-write). When a thread is trying to write, they first create an upgrade_lock with the shared mutex as input and then use upgrade_to_unique_lock to get exclusive access. Here the upgrade_lock is passed as a reference, so I thought maybe `(void) lock` was doing the instantiation or some side effect that it's required to get upgradable access – Laura Rozo Jun 14 '17 at 23:04
  • Then post a link to it!!! –  Jun 14 '17 at 23:06
  • I posted the link on my second comment. Here it is again: [linktoexample](https://stackoverflow.com/questions/989795/example-for-boost-shared-mutex-multiple-reads-one-write) – Laura Rozo Jun 14 '17 at 23:08
  • There is no `(void)` cast in that linked question. –  Jun 14 '17 at 23:10
  • That's why I said I was not 100% sure. I assumed that `(void) lock` was getting the upgradable access to the shared mutex, as in this line on the example: `boost::upgrade_lock lock(_access);`. I didn't write this code, I'm just trying to understand what is doing – Laura Rozo Jun 14 '17 at 23:19
  • I really have no idea why you would assume that - the line you quote doesn't do a void cast. A void cast is basically a no-op that keeps the compiler quiet about things like the return value of printf. –  Jun 14 '17 at 23:23
  • I'm just trying to figure out what's happening in that code. I thought it was possible it was an overloaded operator that was doing something on the background that was doing something implicitly (aka, assignment?). I haven't been able to find an explanation, that was just a guess. If you have an explanation for why the line of code is there, it would be helpful – Laura Rozo Jun 14 '17 at 23:32
  • For gods sake, the line of code i.e. `(void) lock;`is not there in the answer you linked to! And neither is any other void cast. –  Jun 14 '17 at 23:37

1 Answers1

0

The (void) lock; condition is present there to avoid compilation errors in the event that something like g++ -DNDEBUG -Wall -Werror -pedantic is used during compilation. This indicates to the compiler to stop on all warnings for unused variables (among other things). It's a technique sometimes used in code when the programmer doesn't know whether the variable will be used, but doesn't want to break the build.

The owns_lock() method indicates that a particular object has acquired ownership of the lock. Or to say that differently, locked the object. If it's a shared lock, then multiple objects can 'own' it. 'owned' is synonymous 'with locked by this object'. If it's a unique lock then only a single object can own it.

The assert() is probably just there for debug builds, without which the code would fall through attempting to silently upgrade the lock to a unique lock; even in the event that the lock is not owned by the thread (e.g. it is unlocked). If this were true, then the upgrade would fail silently.

For example, if some ordering of operations as follows occurred with the assert disabled, the code would silently fail:

boost::upgrade_lock<boost::shared_mutex> lock(this->mainMutex);
lock.unlock();
this->setAttribute(lock, false);

The upgrade_lock call acquires upgrade ownership of the lock. Meaning that there is a contract saying this object can upgrade the lock.

The upgrade_to_unique_lock call does the actual upgrade of the shared lock to a unique lock by stalling until any other shared owners release the lock. If the lock isn't owned by the object when this is called, then it will silently fail to upgrade the lock to a unique lock.

TLDR; I suspect that the authors intention was to keep the mutual exclusion code private to the class, but wanted to put an assert in case unlock() was accidentally called somewhere prior to setattribute().

snaphat
  • 41
  • 1
  • 3