4 ms·
When, how, and why. The biggest part of mutexes and how to properly use them is thinking of the consistency of the data that you are working with. Here's a re
by cogman10 2y ago
When, how, and why.
The biggest part of mutexes and how to properly use them is thinking of the consistency of the data that you are working with.
Here's a really common bug (psuedocode)
if (lock {data.size()} > 0) {
value = lock { data.pop() }
lock { foo.add(value) }
}
The issue here is size can change, pop can change, and foo can change in unexpected ways between each of the acquired locks.
The right way to write this code is
lock {
if (data.size() > 0) {
value = data.pop()
foo.add(value)
}
}
That ensures the data is all in a consistent state while you are mutating it.
Now, what does make this tricky is someone well-meaning might have decided to push the lock down a method.
Imagine, for example, you have a `Foo` where all methods operate within a mutex.
This code is also (likely) incorrect.
value = foo.bar()
if (value.bat()) {
foo.baz(value)
}
The problem here is exactly the same problem as above. Between `foo.bar()` and `foo.baz()` the state of foo may have changed such that running `foo.baz(value)` is now a mistake. That's why the right thing to do is likely to have a `foo.barBaz()` method that encapsulates the `if` logic to avoid inconsistency (or to add another mutex).
In java, the most common manifestation (that I see) of this looks like this
var map = new ConcurrentHashMap();
if (map.get(foo) == null)
map.put(foo, new Value());
Because now, you have a situation where the value of `foo` in the map could be 2 or more values depending on who gets it. So, if someone is mutating `Value` concurrently you have a weird hard to track down data race.
The solution to this problem in java is
map.computeIfAbsent(foo, (unused)->new Value());
- hinkley 2y agoComposing locks is where Java usually blows up. And computeIfAbsent can end up holding the lock for too long if the function is slow.
- foobazgt 2y agoComposing locks isn't a Java problem - it's a fundamental abstraction problem with locks. This is one of the reasons why you usually reach for higher level abstractions than mutexes. > And computeIfAbsent can end up holding the lock for too long if the function is slow. How is this different from any other lock-holding code written anywhere?
- hinkley 2y agoI’m saying Java is exceptionally bad at this because every object is its own mutex. And you end up having to trade single core performance for multi core by deciding to speculatively calculate the object. If there’s no object to make the critical section is very small. But as the object sprouts features you start smashing face first into Amdahl.
- foobazgt 2y ago> because every object is its own mutex. Not true in any practical sense. > And you end up having to trade single core performance for multi core by deciding to speculatively calculate the object. What is the alternative you suggest? If you care about having the predicate actually hold, and you also don't want to have to hold the lock while constructing the object, then you're going to end up in an optimistic-concurrency scenario where you check the predicate under lock, compute the object, and check again before swapping the value in. You may end up having to throw your work away when you discover the predicate changed. Java is no better nor worse at doing this than anything else.
- hinkley 2y ago> Not true in any practical sense. This is going to put a damper on any further conversation. Even with coarsening and elision every synchronized function closes a lock on the enclosing object.
- int_19h 2y agoDo people actually use `synchronized` methods in Java these days? It's been commonly described as an anti-pattern (for all the reasons discussed upthread here) two decades ago already.
- hinkley 2y agoThe more useful question is has it been expunged from the JDK and common libraries. I think it's been more like 10-12 years since it really started being talked about in more than certain subcommunities and that's almost 20 years' worth of existing libraries. OpenTelemetry is a fairly recent library. Even if you ignore some test fakes (where, let's face it, who cares), it still uses it in a few places, and uses lock objects in others. I don't see much evidence of recursion going on with the former. But that's how things always start and later there's running and screaming.
- another-acct 2y agoMight want to move foo.add() out of the lock scope (assuming foo is a thread-private resource): value = nil lock { if (data.size() > 0) { value = data.pop() } } if (value) { foo.add(value) }
- samatman 2y agoThis is one of the areas where Zig's combination of anonymous blocks and block-based defer really pay off. To create a locked region of code is just this { mutex.lock(); defer mutex.unlock(); // Do mutex things } It's possible to get this wrong still, of course, but both the anonymous scope and the use of `defer` make it easier to get things right. Nothing can prevent poor engineering around mutex use though. I'd want a critical path for a concurrent hashmap to look like this: { shared_map.lock(); defer shared_map.unlock(); if (shared_map.getOrNull(foo) == null) { shared_map.put(foo, new_val); } } Where the SharedMap type has an internal mutex, and a way to check it, and all operations panic if no lock has been acquired. It could have `shared_map.lockAndGet(OrNull?)(...)`, so that the kind of problem pattern you're describing would stand out on the page, but it's still a one-liner to do an atomic get when that's all you need the critical path to perform. I don't think these invariants are overly onerous to uphold, but one does have to understand that they're a hard requirement.
- int_19h 2y agoThis doesn't seem to add anything over and above what std::mutex in C++ or a synchronized block in Java offer?
- senderista 2y agoLess than C++. defer() is strictly inferior to RAII.
- jhatemyjob 2y agoI digress but my autistic brain couldn't help itself. Provided that it's a recursive lock you could do this instead of adding a new method `foo.BarBaz` foo.lock { value = foo.bar() // foo.lock within this method is ignored if(value.bat()) { foo.baz() // foo.lock within this method is ignored } } Also, to catch this bug early, you could assert foo is locked in `value.bat` or something. But that may or may not be feasible depending on how the codebase is structured