How does Herb Sutters monitor class work?

Question

I am trying to understand the monitor class that Herb Sutter presented on C++ and Beyond 2012:

template<typename T>
class monitor {
private:
    mutable T t;
    mutable std::mutex m;

public:
    monitor(T t_ = T{}) : t{ t_ } {} 
    template<typename F>
    auto operator()(F f) const -> decltype(f(t))
    {
        std::lock_guard<std::mutex> _{ m }; return f(t);
    }
};

I have managed to create a class that does the same thing in a more old fashioned and simpler (for me at least) way:

template<typename T>
class MyMonitor {
public:
    MyMonitor() { t = T(); }

    template<typename F>
    auto callFunc(F f) {
        std::lock_guard<std::mutex> lock(m);
        return f(t);
    }

private:
    T          t;
    std::mutex m;
};

In which ways are Herb Sutters code better than mine?

There is fundamental difference, your class doesn't handle properly reference returnd by functor `F`. You just discard it. — rafix07, Mar 04 '20 at 09:04
Don't understand. Are you saying that if a functor f returns a reference my monitor will not work? — Andy, Mar 04 '20 at 09:08
Using a constructor initialiser list, where possible, is considered preferable over assignment in the constructor body, since it involves constructing less objects. Point to Herb. Being able to use a object specified as `const` is preferable to being unable to use it. Point to Herb. Being explicit about the return value of `operator()` being the type returned by `f()` means less potential confusion for the programmer about the return type. Point to Herb. Being able to properly handle functors that return a reference. Point to Herb. Using variable named `_` hurts readability. Point to you. — Peter, Mar 04 '20 at 09:08
Note that `auto` functions without a trailing return type were added in C++14 (so they aren't that old-fashioned), and follow the normal rule that `auto` is never a reference. — molbdnilo, Mar 04 '20 at 09:17
@Andy Yes, exactly. Your `operator()` will return by value if the functor returns by reference. You can use `decltype(auto)` as return value to get the same behaviour as Herbs code without trailing return type. — super, Mar 04 '20 at 09:34
Theres a much nicer way to write a monitor class than Herbs idea (at least its nicer to use) using a wrapper monitor: https://stackoverflow.com/questions/12647217/making-a-c-class-a-monitor-in-the-concurrent-sense/48408987#48408987 — Mike Vine, Mar 04 '20 at 10:32
@Peter: regarding your first point: yes I see now that where Herb calls a copy ctor I call a ctor + an assignment operator. Thanks. Silly mistake. I would replace this by: MyMonitor() : t(T()) { ; } where I use one temporary object less than Herb. Herb on the other hand supports adoption which is probably useful. — Andy, Mar 04 '20 at 10:35
@MikeVine: I prefer this one which allows to "protect" any block of code. whereas the `operator->` only allow to protect a method call (so cannot protect 3 `push_back` in a row). — Jarod42, Mar 04 '20 at 10:39
@Jarod42 Did you read the answer? It lets you do it either way - you can protect a block very easily. See the part about `lockedHandle`. — Mike Vine, Mar 04 '20 at 10:41
In fact, if you wanted to you could combine both answers and add the lambda syntax to my wrapper monitor class and then you can pick and choose your ways of accessing in a thread safe manner. — Mike Vine, Mar 04 '20 at 10:43
That answer allows universal way. Your `ManuallyLock` seems so old fashioned (extra (unnatural) scope). Lambda scopes are "automatic". — Jarod42, Mar 04 '20 at 11:02
@Peter: If I rewrite to use mutable I can do this: std::vector ivec; const MyMonitor> ivecm(ivec); ivecm.callFunc([](std::vector &ivec) { ivec.push_back(1); }); Does this make sense? I can declare something as const and then still change it? — Andy, Mar 04 '20 at 13:27

Jarod42 · Accepted Answer · 2020-03-04T10:32:11.050

3

In which ways are Herb Sutters code better than mine?

Your T should be default constructible, and assignable.
In Herb Sutters code, T should be copy constructible.
Herb Sutters code allows to initialize the member.
Your operator () doesn't handle reference.

edited Mar 04 '20 at 10:32

answered Mar 04 '20 at 09:09

Jarod42

203,559
14
181
302

also OPs `operator()` is not const-correct, but I guess thats the minor issue – 463035818_is_not_an_ai Mar 04 '20 at 09:10
2

@idclev463035818: `mutable` and `const` versus not `const`... – Jarod42 Mar 04 '20 at 09:11
yes, what I meant is that it should be `const` but isnt, which prevents to use a `const monitor` when it should be possible – 463035818_is_not_an_ai Mar 04 '20 at 09:13
2

*"it should be const"*. Not sure, whereas `mutex` indeed doesn't change const semantic of the object, modifying `T` is more controversal IMO. – Jarod42 Mar 04 '20 at 09:17
Are you sure about the second point? I wrote a Dummy class that prints out inside constructor etc. Herbs code: Dummy ctor, Dummy copy ctor, Dummy dtor. – Andy Mar 04 '20 at 10:39
@Andy: Try to use `struct S { S(int) {} };` And then `monitor ~~m(42);`~~ – Jarod42 Mar 04 '20 at 10:43
ah ok, understand your point. Depends on what semantics `monitor` should offer, both choices are "correct" – 463035818_is_not_an_ai Mar 04 '20 at 11:07
Do not understand your point about semantics. If I rewrite to use mutable I can do this: std::vector ivec; const MyMonitor> ivecm(ivec); ivecm.callFunc([](std::vector &ivec) { ivec.push_back(1); }); Does this make sense? I can declare something as const and then still change it? – Andy Mar 04 '20 at 13:29
1

**Reading** a synchronized resource which require to lock `mutex` should not be`const`, because of the `mutex`, but it might be seen as `const` semantically as you don't modify the resource itself. so `mutable` `mutex` is ok, but as I said `mutable` resource is controversial. Ideally, we want 2 overloads, one const and one not. – Jarod42 Mar 04 '20 at 13:47

How does Herb Sutters monitor class work?

1 Answers1