10 replies to this topic

[Wernaeh]

Hi there again =)

When playing around a bit with a multithreaded application (one logic thread, and one physics thread), I came upon a situation where I'd need to use a mutex on a constant member function to keep the call's result consistent.

class foo

{


private:


    // Mutual class access

    CMutex mutex;


    // Example members

    // May be changed at any time by another thread

    int a;

    int b;


public:


    // Access calls, used by another thread

    void SetA(int in_a)

    {

         mutex.Acquire();

         a = in_a;

         mutex.Release();

    }


    void SetB(int in_b)

    {

         mutex.Acquire();

         b = in_b;

         mutex.Release();

    }


    // Constant observer call

    // Needs the mutex so a and b are not changed while placing

    // them into the storage object

    void GetFooStuff(stuffstorageclass &stuffstorage) const

    {

         // Gives a compile error, since the call is const

         mutex.Acquire();


         stuffstorage.a = a;

         stuffstorage.b = b;


         // Dito

         mutex.Release();

    }

}

The function itself is a simple observer call, which fills a structure passed in via parameter with values from the object instance. The appropriate values may also be changed at any time because of another thread. Consequently, I use a mutex to synch access to the instance.

However, since the member function is just an observer, I'd hate to make it non-const. Yet, the mutex is a member variable too, and needs to be set.
I can't make the mutex const for obvious reasons.

Now, long text, small question =)

Is this (general) situation a good place to use the "mutable" keyword, to allow mutexing for observer calls ? I'm just asking because I heard that mutable variables are a sign of bad code layout.

If so, are there any obvious improvements to the code above I might have overlooked ?

Thank you for your time,
Cheers,
- Wernaeh

[eddie]

I believe in this case you're totally correct in making the mutex mutable.

Terrible usage of mutable comes into play when you're putting it on things that muck with your domain. The mutex is more of an access-control object, which has no role other than to protect data from being mishandled.

Just my two cents. :)

[Wernaeh]

Ok.. thank you very much for your input =)
I already feel kinda relieved ;)

Cheers,
- Wernaeh

[corey]

Considering what it is, that's probably the least offensive use of mutable that you can do.

Then again, you could consider if it is really neccessary and if it would confuse any other programmers working on the code.

The problem with mutable is a public member that can be changed without warning or error even when the class is instantiated as a const object! If you have a lot of library or API hidden code for a reason or are passing around a lot of member pointers, this is where it can get messy.

I personally don't use it in any code right now.

corey

[.oisyn]

I think a more interesting question would be whether the methods Acquire and Release of CMutex should be const or not. Should you be able to acquire and release const mutexes? Is it really a change of the mutex object? Then again, why would you ever bother making a mutex const, other than as a member of another const object (such as the situation in this topic)? So in that retrospect, making the methods const would do you more good than harm.

Any thoughts?

[monjardin]

Make a class to manage the mutex locking/unlocking:

class CMutexLocker
{
CMutex& mutex;

public:
CMutexLocker( CMutex& m ) : mutex( m ) {
mutex.Acquire();
}
~CMutexLocker() {
mutex.Release();
}
};

Now your const function looks like this:

void GetFooStuff(stuffstorageclass &stuffstorage) const
{
// Acquires on construction and releases on destruction
CMutexLocker locker( const_cast<foo*>(this)->mutex );

stuffstorage.a = a;
stuffstorage.b = b;
}

This not only eliminates your const problem, but it is more exception safe as well. If for you had code throw an exception in between a lock and release, the mutex would never be released. In this case, a local class destructor gets called even when an exception is thrown.
Now you don't have to worry about whether or not to use mutable!

-Judge

Edit 1: Fixed CMutexLocker to use a reference to a CMutex.
Edit 2: Put the const_cast in GetFooStuff.

[.oisyn]

You're depending on the fact that a CMutex is copyable. What if it isn't?

[monjardin]

Sorry, that should have been a reference:

class CMutexLocker
{
CMutex& mutex;

public:
CMutexLocker( CMutex& m ) : mutex( m ) {
mutex.Acquire();
}
~CMutexLocker() {
mutex.Release();
}
};

I'm not being very thorough here, but the idea is sound. Take a look at boost::mutex::scoped_lock for more ideas:http://www.boost.org...epts.ScopedLock

-Judge

[.oisyn]

But that won't work since the mutex member you're passing to the constructor is const. And if you make it a const reference the Acquire and Release methods should be const, which brings us back to my question earlier: should they be const? And if they were const, the topicstarter wouldn't have the problem in the first place .

Don't get me wrong though, I agree that you should use a guard for exception- and error-safe programming

[monjardin]

You got me again! =) So... I broke down and actually tested my code.
He could do this using the same CMutexLocker class and still remove the need for mutable:

void foo::GetFooStuff(stuffstorageclass &stuffstorage) const
{
CMutexLocker locker( const_cast<foo*>(this)->mutex );

stuffstorage.a = a;
stuffstorage.b = b;
}

Which is the lesser of two evils, mutable or const_cast?
And thanks for keeping me honest .oisyn! ;)

-Judge

[Wernaeh]

Nice to see so many followups here =)

I finally decided that I could go with the mutable keyword, refraining from casting the const away of the instance, mainly due to code readability. (Also I'm not too sure what an optimizing compiler could do to recasting the this pointer to nonconst in a const member function, especially when changing anything at that new pointer)

I already had a CMutexCall object programmed which essentially did the same as the proposed CMutexLocker, but it is a useful technique for mutexing calls, nevertheless.

I also considered adding const to the AcquireMutex and ReleaseMutex member calls. Yet I dismissed the idea, since the actual, encapsulated mutex object is changed by acquire / release more directly than via locking the mutex from some other const function.

Thank you for your answers.

Cheers,
- Wernaeh