Brian Lovin
/
Hacker News
Daily Digest email

Get the top HN stories in your inbox every day.

comex

This is how Rust’s mutexes work as well - albeit using RAII rather than a callback, but with the same fundamental innovation of putting the data inside the mutex object. I’ve used it in my C++ code as well; it’s a great pattern.

tialaramex

Crucially the Rust borrow checker means using this wrong from (safe) Rust is impossible, whereas it's relatively easy to use either C++ approach wrongly, especially if nobody has actually sat down and explained why you're doing it. In C++ we can "just" leak the reference to the protected data, in (safe) Rust of course the borrow checker means that programs with this mistake won't build.

Rust's ZSTs (~ unit types have zero storage size) make it reasonable to write Mutex<MyMarker> and then require that people show you their MyMarker (which they can only get at by holding the lock) to do stuff thus enforcing locking even when it's not really data you're protecting. Because these are zero size, the resulting machine code is unchanged but the type checking means if you forget to lock first your program won't even build, which shifts locking mistakes very hard left.

chrisshroba

Can you give an example of using a ZST as a marker in a Mutex? I think I understand that you're suggesting this as a way of locking some section of code even if it's not a specific piece of data that you're locking, but I'm wondering how this could "enforce locking" then, since anything you do while locked could just as easily be accidentally done without the lock, right?

tialaramex

Sure:

https://rust.godbolt.org/z/74KT3Tcaa

The idea is we wrap code that ought to only run with a lock as a function, and we define the function such that one of its required arguments is a reference to the Marker type. When somebody tries to write code to call these functions, they're reminded by its signature that they'll need the Marker type, and the only way to get it is to lock some Mutex provided for the purpose.

You're correct that if they're able to do all the stuff which needs a lock themselves without calling these functions, they can hurt themselves, and if that's easy enough they might do it by accident. However it's not often that we've got something that's dangerous enough to need protecting this way and yet also so easy that people would rather re-implement it wrongly than read how to take the lock.

Rust's POSIX stdio features are protected this way, and obviously on some level you could bypass all that, and unsafely cause write syscalls with the right file descriptor number but like, I'd guess it'd take me an hour to write all that, whereas println!() and co. are right there in front of me, so...

Georgelemental

You would have an `fn do_something_that_requires_held_lock<'a>(marker: ZstMarker<'a>)`. To call the function, you need a value of type `ZstMarker<'a>`, and you would structure your API such that the only way of getting that value is by locking the mutex.

Crucially, the lifetime parameter `'a` ensures that the marker can't outlive the muted being unlocked.

One example of this pattern is the `Python<'py>` marker in the `pyo3` library (https://docs.rs/pyo3/latest/pyo3/marker/struct.Python.html), which represents holding the Python GIL. The internals of `pyo3` do lots of `unsafe` FFI with the Python runtime, but the API exposed by the library is safe thanks to this pattern.

nextaccountic

But is there a way to use Rust's approach of RAII guards in C++, rather than accessing the mutex inside a closure?

gpderetta

Of course. Boost.synchronized and the folly variant have been mentioned elsethread.

ReactiveJelly

Adding my favorite to the pile: https://github.com/dragazo/rustex

MuffinFlavored

> RAII

Resource acquisition is initialization

pitaj

This is one of those cases where the acronym itself carries more meaning than the expansion.

Blackthorn

Neat idea. Though I think having to pass a lambda for anything you want to do with the fields is awful ergonomics.

Maybe instead combine the two ideas. MutexProtected<T> but can be locked with MutexProtected<T>::lock() which returns a MutexLocked<T> object. That object then cleans up the lock when it goes out of scope, and also provides direct access to the enclosed type.

mchicken

FpUser

No, but Boost is one giant bear of dependency with lots of duplication with STL. Personally I prefer to use tiny includable file / lib for specific functionality that I need rather than keep that bear on my real estate.

zbentley

This common criticism conflates two things.

Boost is not monolithic, nor are any of the sub-parts especially huge (like ... asio is big because an asynchrony system is a big project; regex is small because it can be). Nor, as other commenters have pointed out, is including those dependencies particularly onerous; many are single-file or just a handful of files.

I suspect your criticism has more to do with "one giant bear of dependency". Which: fair enough. Getting Boost set up is a pain; the pain does not reduce if you only want to use one sub-part of it, because you still have to use b2, their custom build language and associated cmake-ish system (yuck!) to prepare it (there are ways to avoid this, but they are not well-supported). Even CMake's (pretty good!) Boost discovery/inclusion support can only conceal that weirdness to a point, and often breaks in unexpected environments--well, breaks more often than the average CMake setup, which breaks a lot...

Similarly, getting your IDE to understand boost's presence can be tricky since many IDEs (looking at you VS) have overly-opinionated ways of incorporating it that don't jive well with how a CLI/portable project build system pulls it in.

But the first and second paragraphs aren't the same thing. Initial setup pains are real and suck, but the initial setup pains and the overhead/monolithic-ness of actually using boost in your projects once it's set up are another. Initial setup pain is largely a one-time or infrequent cost, and the infrequency of that cost should be weighed against the not-inconsiderable convenience of Boost as a tool.

Not saying it's universally worth it; for some projects (especially ones that need to be built on a wide variety of environments, though this is rarer than most people, even some project authors, think) it's not appropriate. But many of the standard criticisms of Boost's runtime utility are specious.

planede

Boost is multiple libraries, you don't have to use all of it. Many of it are header-only as well.

ot

That's how folly::Synchronized works, it supports both the callback interface as MutexProtected, and an interface that returns a RAII lock holder that gives access to the underlying object. It is generally the preferred synchronization utility at Meta and thus it is widely used.

https://github.com/facebook/folly/blob/main/folly/docs/Synch...

biesnecker

As someone who had to occasionally write C++ in FBCode but not enough to really feel comfortable with it, `folly::Synchronized` and its straightforward semantics were really nice.

dietr1ch

When using locks I'm more paranoid about misusing the lock and don't mind typing a bit more to make the code obvious. Also, it seems that good auto-completion for that pattern is simple to achieve.

Alternatives to make sure you are not grabbing the wrong lock include this much uglier GUARDED_BY macro, - http://clang.llvm.org/docs/ThreadSafetyAnalysis.html

I'd say the extra lambda is a fair price to pay

Someone

It’s a fair price to pay, but I think the suggested change is better because it makes you pay less for the same benefit. It still would be obvious that there’s a lock, but the code would IMO be simpler:

  thing->field.with([&](Field& field) {
    use(field);
  });
would become

  {
     auto field = Mutex.locker(thing.field); // or thing.field.lock();
     use(field);
  }
or

  use(Mutex.locker(thing.field));
Yes, that requires you to understand destructors exist, but if you’re programming C++, that’s a given.

I find that easier to understand (partly because the first doesn’t mention ‘lock’. I don’t think ‘with’ is the best name there)

simplotek

> because it makes you pay less for the same benefit.

Requiring a lambda capturing stuff and be passed around is not what I would call "pay less", when the alternative is just adding a block and instantiate a std::lock_guard

akling

Yes, that's a great way to improve the ergonomics of MutexProtected in simple cases! We should totally add that as a complement to the lambda API. :)

akling

After sketching this out, it's nice that this:

    auto x = state.with([](auto& state) { return state.x; });
Becomes this:

    auto x = state.locked()->x;
But it also creates this very accessible footgun:

    auto& x = state.locked()->x;
Where it's way too easy to bind a reference to something that should be protected by the lock, but now isn't. So I'm not sure this is a great idea anymore.

gpderetta

At the end of the day, this is C++, there are no compiler tracked lifetimes and it is easy to leak references out of the lambda as well.

At least the synchronized pattern makes it easy to document which state is protected by which mutex.

Blackthorn

> But it also creates this very accessible footgun:

Well, you said it yourself in the article though. There's always ways around the locking; C++ doesn't really give you the full ability to guarantee a field is locked when access. You'll need to either trust the users to some extent or use a style guide to disallow the pattern (I'd suggest only allowing use of auto x = state.locked() to avoid lifetime questions around state.locked()->x). You'd need to use compiler annotations to get any better.

kajaktum

Wait why is this a footgun?

avidphantasm

Yeah, the syntax/ergonomics is tortured. This is my problem with modern C++ in general. As someone who isn’t a full-time C++ programmer (but who maintains a library written in C++), I have to shy away from anything too fancy as the syntax is too baroque and hard to recall.

simplotek

> Yeah, the syntax/ergonomics is tortured. This is my problem with modern C++ in general.

I disagree. I mean, the ergonomics in this case are indeed awful, but this has nothing to do with C++, modern or not. This is an API design problem, possibly made worse by people trying to be too clever for their own good.

somerando7

IMO passing a lambda for synchronized code makes it much easier to read (going off of working with folly::Synchronized)

simplotek

> Neat idea.

I don't agree. It sounds like a higher level path to deadlocks.

> Maybe instead combine the two ideas. MutexProtected<T> but can be locked with MutexProtected<T>::lock() which returns a MutexLocked<T> object. That object then cleans up the lock when it goes out of scope, and also provides direct access to the enclosed type.

It sounds like you're trying to invent std::lock_guard with extra steps.

https://en.cppreference.com/w/cpp/thread/lock_guard

jmull

> It sounds like ... std::lock_guard with extra steps.

I'm not the previous poster, but I think they are still suggesting that the value not be accessible until the lock is acquired.

Something like this (Apologies, I haven't done C++ is a long time and this is just off the top of my head. It's based on the example from the lock_guard example for the sake of comparison):

    guardthing<int> guarded_value;

    void safe_increment()
    {
        auto lock = guarded_value.lock();
        lock.value += 1;

        std::cout << "value: " << value << "; in thread #"
                  << std::this_thread::get_id() << '\n';
     
        // the mutex in guarded_value is automatically released when lock
        // goes out of scope
    }

Blackthorn

> It sounds like you're trying to invent std::lock_guard with extra steps.

No, because lock_guard doesn't guard fields. lock_guard is simply the RAII option they were talking about in the article.

loeg

> MutexProtected<T> but can be locked with MutexProtected<T>::lock() which returns a MutexLocked<T> object. That object then cleans up the lock when it goes out of scope, and also provides direct access to the enclosed type.

In fact, MutexProtected provides ::lock_exclusive() and ::lock_shared() methods which do exactly that. The article just fails to mention them. https://github.com/SerenityOS/serenity/blob/master/Kernel/Lo...

boricj

Another feature of the MutexProtected implementation in SerenityOS is the ability to either obtain an exclusive lock (writable) or a shared lock (read-only). A shared lock provides a const reference to the protected value and therefore enforces read-only access semantics at compile-time by leveraging the C++ type system (specifically, const-correctness).

The initial PR introducing the ancestor to MutexProtected has some details on the motivation behind it and also unearthed a bunch of incorrect locking that the C++ compiler caught when introducing it: https://github.com/SerenityOS/serenity/pull/8851

afc

I also use this pattern: https://github.com/alefore/edge/blob/master/src/concurrent/p...

I added an optional template Validator parameter that can be used to double-check invariants on the protected fields.

I also declared a Protected with condition subclass. Here is one example of how I use it to implement a "barrier" of sorts (the operation instance blocks in the destructor until execution of all callables passed to "Add" has completed): https://github.com/alefore/edge/blob/master/src/concurrent/o...

I found it slightly preferable over Abseil annotations; I think it's slightly more robust (i.e., make errors, like forgetting an annotation, less likely) and I like making this explicit in the type system.

vore

Clang’s static thread safety analysis is pretty good for this too: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html

mike_hearn

Yep, it's a useful pattern albeit not a game changer or anything. I've been using it in Kotlin for a while. Here's an implementation:

https://gist.github.com/mikehearn/1913202829403f65331123f047...

One of the nice things about doing it in Kotlin rather than C++ is the clean syntax using trailing lambda blocks and anonymous objects:

    private val state = Locker(object {
        var value = 1
        var another = "value"
    })


    val nextValue = state.locked { ++value }
This works because Kotlin lets you define anonymous types that can take part in type inference even though they're not denotable, and then use them as lambda receivers. So "this" inside the final block points to the unnamed singleton.

It has a few nice features:

• It's runtime overhead free. Kotlin frontend can inline the "locked" call so you end up with the equivalent of manual locking.

• There's no way to get to the state without holding the lock unless you deliberately leak it outside the lambda.

• You can control re-entrancy.

• It tells the compiler the lambda is only invoked once, which has various benefits for allowing more natural code constructs.

These days I'd probably code it with an explicit ReadWriteLock so it's Loom compatible and to allow explicit read/write blocks, but the JVM optimizes locks pretty well so if there's never any contention the memory for the lock is never allocated. I'd also experiment with making it value type so the overhead of the Locker object goes away. But it was never necessary so far.

quietbritishjim

[edit: deleted first half of my comment - it was just duplication of existing top comment]

-----

Even better, when possible (it isn't always), is to avoid using mutexes at all, and instead have the data structure owned by a relevant thread. When you need to access a data structure, pass a message to that thread, and then just access the data structure freely when you get there. (Of course the inter thread queue uses a mutex or some other synchronisation mechanism, and the queue itself effectively acts as a synchronisation mechanism.)

    struct SomeMsg {
        SomeMsg() {
            thread_foo->pass_message(std::bind_front(this, &SomeMsg::handle_foo));
        }
        void handle_foo(Foo& foo) {
            // ... use foo ...
            thread_bar->pass_message(std::bind_front(this, &SomeMsg::handle_bar));
        }
        void handle_bar(Bar& bar) {
            // ... etc. ...
        }
    };

kps

Chrome does this, more or less, with different syntax. https://chromium.googlesource.com/chromium/src/+/main/docs/t...

smallstepforman

You’ve just described Actors. Now take the next step and add a work stealing schedular for a fuller Actor system.

MontagFTB

I’ve been toying with these lately, too. My solution was to give each actor an executor where it will run:

    actor<int> foo(stlab::default_executor, “name”, 42);
    auto f = foo.send([](auto& x){
        std::cout << x << ‘\n’; // prints 42
    });
Each call to ‘send’ returns a stlab::future which can have a continuation chained, etc.

The nice thing about these is the executor based interface is generic. The actor could be dedicated to a thread, run on an OS-provided thread pool, etc.

At the time the actor is running, the thread it is on is temporarily given the name of the actor to ease debugging.

quietbritishjim

Almost. If each thread were an Actor (I think that's what you're saying?) then that would structure the code around each type of work e.g. here is the code for all the database operations. That's not a very useful way to group things for the developer. The way I wrote it above, the code is structured around a single activity across all the types of work it has to do.

loeg

> (Of course the inter thread queue uses a mutex or some other synchronisation mechanism, and the queue itself effectively acts as a synchronisation mechanism.)

I'd suggest using MPSC queues for this purpose (assuming the recipient is a single thread rather than a pool).

nly

Or...you could use Clangs GUARDED_BY annotations and thread safety analysis...

https://clang.llvm.org/docs/ThreadSafetyAnalysis.html

franke2

Even if very similar, I prefer the MutexedObj class described in those two articles

https://fekir.info/post/extend-generic-thread-safe-mutexed_o...

https://fekir.info/post/sharing-data-between-threads/#_bind-...

In particular it explains why providing getters or operator* like some other implementation do (not in this case) is not a feature, and it does not use multiple classes (like the linked implementation of SerenityOS does) making the implementation simpler.

Also the "simple" implementation is just 20 lines of code...

loeg

folly::Synchronized<T> provides a similar pattern. It works well for very simplistic concurrency schemes.

  struct { int a; int b; } A;

  folly::Synchronized<A> syncA;

  int c;
  {
    auto aReader = syncA.rlock();
    c = aReader->a;
    // aReader going out of scope drops the rlock
  }
  {
    auto aWriter = syncA.wlock();
    aWriter->b = 15;
    // aWriter going out of scope drops the wlock
  }
I would argue that the pseudo-pointer accessor objects are more usable than passing lambdas to a `with()` method, but if you want that, folly::Synchronized provides similar `withWLock()` and `withRLock()` methods.

And in fact, MutexProtected seems to provide APIs like Synchronized's `rlock()`/`wlock()`: `lock_shared()` and `lock_exclusive()`: https://github.com/SerenityOS/serenity/blob/master/Kernel/Lo... I don't know why the article doesn't touch on these.

https://github.com/facebook/folly/blob/main/folly/docs/Synch...

https://github.com/facebook/folly/blob/main/folly/Synchroniz...

planede

tialaramex

It got added to the Concurrency TS during the C++ 23 process. It's not in standard C++ itself.

planede

Ah, OK.

Daily Digest email

Get the top HN stories in your inbox every day.

MutexProtected: A C++ Pattern for Easier Concurrency - Hacker News