Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

When thinking about testability and modular code, I recently thought about whether I should split out the synchronization part of a class from the actual behavior part.

By example:

The app "needs" this:

class Mogrifier { // Mogrifier accesses are thread safe
private:
  unsigned int m_state;
  mutex m_sync;
public:
  Mogrifier()
  : m_state(0)
  { }

  unsigned int Mutate() {
    lock_guard<mutex> lock(m_sync);
    const unsigned int old = m_state;
    m_state += 42;
    return old;
   }

  :::
};

Seems simple enough, except that locking and the actual stuff the class does aren't really related.

So I though about whether it's a good idea to split this up:

class Mogrifier { // Mogrifier is *not* synchronized
private:
  unsigned int m_state;
public:
  Mogrifier()
  : m_state(0)
  { }

  unsigned int Mutate() {
    const unsigned int old = m_state;
    m_state += 42;
    return old;
   }

  :::
};

class SyncedMogrifer {
private:
  mutex m_sync;
  Mogrifier m_;
public:
  unsigned int Mutate() {
    lock_guard<mutex> lock(m_sync);
    return m_.Mutate();
  }
};
  • Would you do this?
  • Will it help with unit testing?
  • I'm awful with Pattern names ... what is it called?
  • Is there a simple way to "generate" such a wrapper in C++ for an arbitrary calls?
share|improve this question

2 Answers 2

You really need some assistance from Herb. He can tell you how to deal with this. Cliffnotes:

template<typename T> class Monitor {
    T t;
    std::mutex m;
public:
    template<typename F> auto operator()(F&& f) -> decltype(f(t)) {
        lock_guard<mutex> lock(m);
        return f(t);
    }
};
Monitor<Mogrifier> safe;
safe([](Mogrifier& m) {
    m.mutate();
});

The second part is that you really shouldn't block, and if you do, you can't do it on a per-method level.

share|improve this answer
    
So is it called a Monitor then? –  Martin Ba Feb 15 '13 at 10:49
    
And incidentally I already watched that video. Maybe twice. And I-am-not-blocking according to Herb's interpretation. Because I do not "block", I just take an internal lock for a very short time. (Just like, shared_ptr has to do for example.) And of course I can and should do it on a per method level, as long as I don't "block". (Of course in the specific case of my m_state above I could have used an atomic, but that not always possible when method-internal sync is appropriate.) –  Martin Ba Feb 15 '13 at 10:54
    
@MartinBa: There is a significant difference between the atomics used by shared_ptr, and really holding a mutex. For example, locking a mutex under contention involves a context switch, which is a serious performance problem. Either you use lockfree operations, or you don't, and in this case, you don't (and shared_ptr almost certainly does). –  DeadMG Feb 15 '13 at 14:05
    
Right. shared_ptr can be written with atomics and doesn't need a mutex. Still doesn't change what I said. If you need a mutex (say because the internal mutating state is a vector), then of course you can use a per-method lock for all methods that touch that data. And that is not blocking, that's just what's necessary to keep the data consistent. –  Martin Ba Feb 15 '13 at 14:17
    
@Martin: It is blocking if you make a blocking call, which locking a mutex is. Using a mutex is always blocking. And per-method is horrifically inefficient at best, and can easily also be very incorrect. –  DeadMG Feb 16 '13 at 17:06

I would do this if I had other classes that I wanted to synchronise. If I didn't then I would argue that it's needless complexity and the synchronised mogrifier looks good to me as it is.

It will help with unit testing if you want to test your synchronise code separate from the Mogrifier functionality, and vice verse. If not then it won't. Probably a good idea to unit test your synchronisation code as it's very easy to stuff up.

As for the last two questions... dunno :)

(Oh and BTW I'm Australian so I'm going to be bloody minded and spell synchronise with a S and not a Z even though chrome has highlighted all 5 instances of it in this textarea.)

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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