84

Not sure if this is a style question, or something that has a hard rule...

If I want to keep the public method interface as const as possible, but make the object thread safe, should I use mutable mutexes? In general, is this good style, or should a non-const method interface be preferred? Please justify your view.

Marcin
  • 12,245
  • 9
  • 42
  • 49
  • 2
    See [here](http://www.idinews.com/quasiClass.pdf) for what I think about getters. – sbi Nov 08 '10 at 19:48
  • @sbi I don't understand the point of the paper, as the examples don't show much. If there is a value that needs to be protected by a mutex, for instance, I make a getter and a setter that do nothing but retrieve and set these variables with the help of a mutex. What is wrong with this approach? – San Jacinto Nov 08 '10 at 19:57
  • 1
    @San: The problem is wanting to disclose a private variable in the first place. Methods should be *actions*. Getting and setting are usually poor actions (although sometimes necessary). – Alexandre C. Nov 08 '10 at 19:59
  • @Alexandre that's what I'm getting at. Sometimes you just _need_ a value to be maintained by a class (now that I think about it, it's usually a getter OR a setter, not both). In those cases, the logic to access these is rather cumbersome and dependent upon other things in the class, and I abstract that logic into a getter or setter. Is this an example of what the paper is talking about? – San Jacinto Nov 08 '10 at 20:02
  • @San: My mutexes are used like this: `{ lock l(a_mutex); f(); }`. What would I need a getter for? – sbi Nov 08 '10 at 20:02
  • @sbi I'm not talking about getting the _lock_. I'm referring to a getMySpecialValue() function that guards the value from being updated while I'm in the process of getting it. – San Jacinto Nov 08 '10 at 20:04
  • 3
    @San: Indeed, you often need to compute or read some result from the class. This is not a getter: a `struct process { bool started() const; void start(); };` has neither setters nor getters. Only methods. To the contrary, a `struct employee { int get_salary() const; void set_salary(int); };` makes me puke. – Alexandre C. Nov 08 '10 at 20:07
  • @San: Why do you need to "get" a member data? See Alexandre's comment. – sbi Nov 08 '10 at 20:09
  • @Alexandre So is this paper referring _specifically_ to the case where a developer would use accessor/mutator functions to directly modify a member variable instead of making the member public? Otherwise, I don't follow the argument. – San Jacinto Nov 08 '10 at 20:10
  • @San: Getters and setters are only very marginally better than public data members. If you need them, you're very likely not creating a real _abstraction_ (which is what a class should be), rather than a fancy conglomerate of data (which is what structs were invented for). – sbi Nov 08 '10 at 20:12
  • @sbi I'll give you one instance: I've got a C++ class that uses callbacks frequently. When the data arrives into the callback, I use a polymorphic getter to arrange the data so that the callback can arrange it for the C interface I need to call next, or else I need to implement kernel-level functions in C++, which is not an option. Is this an example of the paper's argument? – San Jacinto Nov 08 '10 at 20:15
  • @sbi That's a given. At times though, you just need to maintain something in a class. In Alexandre's salary exmaple, what is a more appropriate way to store the salary? If I'm writing a frontend to view employee data, don't I need to make an interface in my code to read and set the salary at some point? What replaces this? – San Jacinto Nov 08 '10 at 20:17
  • 1
    @San: `struct employee { int salary() const; void nominate_for_a_raise(); };`. Trust me the secretary using the management program you're writing should not be allowed to call `set_salary`. And you don't want to maintain the ugly permission checking code inside `set_salary`... – Alexandre C. Nov 08 '10 at 20:24
  • 2
    @San: I don't really understand that example, but this isn't important. I, too, sometimes write getters. (I've even written setters.) But _usually_ this is a hint at the design being wrong. – sbi Nov 08 '10 at 20:27
  • @sbi thanks. I'm getting the point of the article more, now. @Alexandre Right, no doubt. At the same time, if one does not build the front-end with the ability for the secretary to call the function, then the problem disappears. What about the case when the head of HR needs to insert a new hire's annual salary? I doubt you want the GUI directly manipulating the database. This implies some temporary storage in memory. Are you saying that this is the purpose of a struct, and not a class? – San Jacinto Nov 08 '10 at 20:33
  • @San: When the HR needs to insert a new hire, it calls a constructor, which has of course a salary argument. I don't make differences between structs and classes. I just want to emphasize the point that there is more beauty and flexibility in methods than accessors. If you want to maintain accessors, fine (I do it sometimes too, but I too take this for a hint at the design smelling). Sometimes they are the right tool for the job (when a public member is not). – Alexandre C. Nov 08 '10 at 20:37
  • @San: I also think (with experience) that interfaces made of accessors tend to change more often that interfaces made of a few well designed methods. – Alexandre C. Nov 08 '10 at 20:39
  • I would appreciate if you guys would stop nitpicking a silly detail that's at best tangentially related, and help address the main question. (It's in the subject.) I will update my wording. – Marcin Nov 08 '10 at 21:02
  • 2
    @Marcin I wasn't nitpicking. I was learning. I'm sorry to have wasted your time on a free site, with free space to ask a question and get free advice in return. Who can I make the check out to? ;) Honestly though, I should have forked it into a new question. I do apologize for clogging your responses. I didn't even think about it. – San Jacinto Nov 08 '10 at 21:55
  • 2
    @Marcin: Be thankful. This must have kept your question on SO's front page for hours. I can see that you still don't have any more answers, but I think this is due to the unclear way you stated your question. As I've said in an earlier comment, I see no need for getters/setters in a mutex, so I have no idea what to answer to your question. Post (a) possible design(s) of your mutex, so that we know what we're talking about. – sbi Nov 08 '10 at 21:58

2 Answers2

74

The hidden question is: where do you put the mutex protecting your class?

As a summary, let's say you want to read the content of an object which is protected by a mutex.

The "read" method should be semantically "const" because it does not change the object itself. But to read the value, you need to lock a mutex, extract the value, and then unlock the mutex, meaning the mutex itself must be modified, meaning the mutex itself can't be "const".

If the mutex is external

Then everything's ok. The object can be "const", and the mutex don't need to be:

Mutex mutex ;

int foo(const Object & object)
{
   Lock<Mutex> lock(mutex) ;
   return object.read() ;
}

IMHO, this is a bad solution, because anyone could reuse the mutex to protect something else. Including you. In fact, you will betray yourself because, if your code is complex enough, you'll just be confused about what this or that mutex is exactly protecting.

I know: I was victim of that problem.

If the mutex is internal

For encapsulation purposes, you should put the mutex as near as possible from the object it's protecting.

Usually, you'll write a class with a mutex inside. But sooner or later, you'll need to protect some complex STL structure, or whatever thing written by another without mutex inside (which is a good thing).

A good way to do this is to derive the original object with an inheriting template adding the mutex feature:

template <typename T>
class Mutexed : public T
{
   public :
      Mutexed() : T() {}
      // etc.

      void lock()   { this->m_mutex.lock() ; }
      void unlock() { this->m_mutex.unlock() ; } ;

   private :
      Mutex m_mutex ;
}

This way, you can write:

int foo(const Mutexed<Object> & object)
{
   Lock<Mutexed<Object> > lock(object) ;
   return object.read() ;
}

The problem is that it won't work because object is const, and the lock object is calling the non-const lock and unlock methods.

The Dilemma

If you believe const is limited to bitwise const objects, then you're screwed, and must go back to the "external mutex solution".

The solution is to admit const is more a semantic qualifier (as is volatile when used as a method qualifier of classes). You are hiding the fact the class is not fully const but still make sure provide an implementation that keeps the promise that the meaningful parts of the class won't be changed when calling a const method.

You must then declare your mutex mutable, and the lock/unlock methods const:

template <typename T>
class Mutexed : public T
{
   public :
      Mutexed() : T() {}
      // etc.

      void lock()   const { this->m_mutex.lock() ; }
      void unlock() const { this->m_mutex.unlock() ; } ;

   private :
      mutable Mutex m_mutex ;
}

The internal mutex solution is a good one IMHO: Having to objects declared one near the other in one hand, and having them both aggregated in a wrapper in the other hand, is the same thing in the end.

But the aggregation has the following pros:

  1. It's more natural (you lock the object before accessing it)
  2. One object, one mutex. As the code style forces you to follow this pattern, it decreases deadlock risks because one mutex will protect one object only (and not multiple objects you won't really remember), and one object will be protected by one mutex only (and not by multiple mutex that needs to be locked in the right order)
  3. The mutexed class above can be used for any class

So, keep your mutex as near as possible to the mutexed object (e.g. using the Mutexed construct above), and go for the mutable qualifier for the mutex.

Edit 2013-01-04

Apparently, Herb Sutter have the same viewpoint: His presentation about the "new" meanings of const and mutable in C++11 is very enlightening:

http://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Community
  • 1
  • 1
paercebal
  • 81,378
  • 38
  • 130
  • 159
  • This is certainly a great explanation, `+1` from me for that. I'm just not sure, though, whether it even addresses the problem of the OP, because his question is so vague. – sbi Nov 09 '10 at 08:30
  • 2
    @sbi : Thanks! Fact is, I was confronted to the same problem, so I felt I had to add some context. The OP's hidden info is that he is using a mutex hidden in the class he wants to protect, but it is not said clearly (I had misread the first answer, and thought Alexandre C. had only suggested using an external mutex, when he wrote in five lines what it took me a book or so to explain, so I am to blame, too)... :-) ... – paercebal Nov 09 '10 at 09:56
  • 3
    Thanks for that Herb Sutter video! Worth the time. – mattypiper Mar 28 '13 at 15:28
  • Note: there are better ways of doing external locking than the simple one you mention. You can have an external lock that does not allow helter-skelter access to the underlying mutex. For instance, this article on boost, about [internal and external locking](https://www.boost.org/doc/libs/1_80_0/doc/html/thread/synchronization.html#thread.synchronization.tutorial.internal_locking.internal_and_external_locking). – jwd Sep 28 '22 at 18:32
40

[Answer edited]

Basically using const methods with mutable mutexes is a good idea (don't return references by the way, make sure to return by value), at least to indicate they do not modify the object. Mutexes should not be const, it would be a shameless lie to define lock/unlock methods as const...

Actually this (and memoization) are the only fair uses I see of the mutable keyword.

You could also use a mutex which is external to your object: arrange for all your methods to be reentrant, and have the user manage the lock herself : { lock locker(the_mutex); obj.foo(); } is not that hard to type, and

{
    lock locker(the_mutex);
    obj.foo();
    obj.bar(42);
    ...
}

has the advantage it doesn't require two mutex locks (and you are guaranteed the state of the object did not change).

Alexandre C.
  • 55,948
  • 11
  • 128
  • 197
  • 1
    I'm a POSIX noob. However, I read that semaphores are implemented as futexes, which maintain a FIFO of processes waiting on the lock. Is it a good idea to be passing this around by value? – San Jacinto Nov 08 '10 at 19:45
  • 1
    @San J. you don't pass the mutexes by value, you pass the property you want to read by value. – Alexandre C. Nov 08 '10 at 19:53
  • Sorry, i see what you mean now. – San Jacinto Nov 08 '10 at 20:04
  • 1
    Alexandre, wouldn't the first line need to be `lock locker(the_mutex)`? (And there are ways to not to have to provide the `mutex` template argument.) – sbi Nov 08 '10 at 21:54
  • 1
    There are other fair uses of `mutable`. Caching the results of non-trivial gets is one huge category of places where mutable is fair. – VoidStar Mar 08 '15 at 10:59
  • 4
    @VoidStar: This is what I mean by "memoization". – Alexandre C. Apr 14 '15 at 19:01
  • 1
    In my opinion, anything that doesn't contain information about the state of the object can be made mutable. For example: a class doing image processing, containing a mutable intermediate buffer (which is kept just to avoid reallocation) used by one of the exposed const methods. – Antonio Nov 28 '17 at 16:56