In "Java collections and concurrency," David MacIver offers some advice that should be fairly obvious to anyone who really thinks about it from the standpoint of "why are there so many fixes to this if there's no problem?"

There are various methods in Collections such as synchronizedList, synchronizedMap, etc. These are for wrapping non threadsafe collections in a way that synchronizes important operations. Don't use them. Ever. In a similar theme, never write code that looks like the following:synchronized(myMap){ doStuffTo(myMap); }Concurrency is not an afterthought. If you're going to be doing concurrent programming you should be using datastructures designed for concurrent use. java.util.concurrent has a number of good ones. Further, you should avoid explicitly synchronizing if at all possible and have your structures be internally threadsafe. If you try to ensure thread safety by synchronizing on the structures you're mutating you will a) Make a mistake. Almost certainly. This will introduce bizarre bugs which you will have a serious headache tracking down. b) Have worse concurrent performance than using a properly designed datastructure - e.g. a ConcurrentHashMap has finer grained locking, so it actually is possible for multiple threads to write to it in a safe manner. c) Have really ugly code with synchronization logic spread all over the place. This is not a minor point - if your threading code is simple, it's much easier to determine if it's correct (although still not easy).

As most of us would probably have to admit that at some point we've written synchronized blocks like the code above... it's good advice to read, put bluntly like that.

As someone who has experienced a), b) and c) I agree wholeheartedly.

Also, instead of having the object state separated to multiple attributes each having only a few possible values, it might be possible to represent the combined value-space with a single int value, which can be set atomically, practically implementing a finite-state-machine with an AtomicInteger or a volatile int accessed/mutated via an AtomicIntegerFieldUpdater holding the state (or an AtomicLong or volatile long accessed/mutated via an AtomicLongFieldUpdater). It can do wonders instead of a locked block or synchronized block for mutating related attributes. Of course it does not apply in all cases.

Has anyone ever tried to build a thead safe business model?

In most cases the business model is stored in the database and I let the database worry about synchronization by setting the correct isolation level or add some explicit locking (pessimistic/optimistic). When all business objects are isolated within a single thread (so a single transaction) you don't need to worry about synchronization on business objects in Java anymore.

Yes. It is actually quite easy to do with some thinking done in advance. The Java Concurrency in Practice book is quite a good read to kickstart that thinking.

Has anyone ever tried to build a thead safe business model?

You mean one were different managers avoid/are not allowed to concurrently give engineers different, conflicting directives based on spur-of-the-moment half baked whims? That would be a "no".

Has anyone ever tried to build a thead safe business model?

You mean one were different managers avoid/are not allowed to concurrently give engineers different, conflicting directives based on spur-of-the-moment half baked whims?
That would be a "no".

LOL - not something I should have read on a conference call (spured by my manager no less). :)

a) Make a mistake. Almost certainly. This will introduce bizarre bugs which you will have a serious headache tracking down. It depends on the situation. If a structure documents that it exposes the internal used lock as an external lock, using synchronized in this manner is a workable solution. If the enclosing structure is owner of the myMap, it is also a workable solution (personally I would add an extra lock instead of using the monitor lock of the mymap). It isn't something I use on a day to day bases btw. b) Have worse concurrent performance than using a properly designed datastructure - e.g. a ConcurrentHashMap has finer grained locking, so it actually is possible for multiple threads to write to it in a safe manner. It depends on the situation (again). If need to do some conditional put for example and you don't want to deal with optimistic locking failures, executing some kind of read and write within a mutex is a workable solution. c) Have really ugly code with synchronization logic spread all over the place. This is not a minor point - if your threading code is simple, it's much easier to determine if it's correct (although still not easy). It depends on the situation (yes.. again). In most cases it is better to use higher abstractions and don't fiddle with locks. But in some cases it is mandatory.

There are various methods in Collections such as synchronizedList, synchronizedMap, etc. These are for wrapping non threadsafe collections in a way that synchronizes important operations.

Don't use them. Ever.

An interesting point. Why would that be true? As long as my concurrent structures are not hit continously but only cautiously the only problem seems to be the performance (b) issue. Yes, it is an issue, but it might not be a massive one. For the other two, using "special" implementations are as succeptible to (a) and (c) as using the wrapped collections.

There are various methods in Collections such as synchronizedList, synchronizedMap, etc. These are for wrapping non threadsafe collections in a way that synchronizes important operations.

Don't use them. Ever.

An interesting point. Why would that be true? As long as my concurrent structures are not hit continously but only cautiously the only problem seems to be the performance (b) issue. Yes, it is an issue, but it might not be a massive one. For the other two, using "special" implementations are as succeptible to (a) and (c) as using the wrapped collections.

In terms of performance, we need to make sure we are talking about the right things. The article uses the phrase "concurrent performance" which is in regards to lock contention. It's not related to the overhead of synchronization. I've never tested this, but I bet you'd find that for single-threaded access and low-contention access, the synchronized map access is faster than access for the respective concurrent collection. The essential thing to know about the synchronized collection classes and wrappers is that it's impossible for these classes to synchronize their iterators. Ever place in code where these Collections and Maps are iterated over needs to be synchronized explicitly. This is where the error come in, especially when you are handing them off to other libraries. On a side note, one way to address this is to invert the iterator pattern and hand a method reference (implemented as a simple interface) to a foreach type method.

Further, you should avoid explicitly synchronizing if at all possible and have your structures be internally threadsafe.

Yea ok, I don't agree with this one at all. The entire point of synchronize is to keep your code block threadsafe. If I could do that without using synchronize, I would, which you can't, which is why you have it. lol

a) Make a mistake. Almost certainly. This will introduce bizarre bugs which you will have a serious headache tracking down.

Yea, but if you don't use it, things aren't any easier. Multi-threading bugs are always going to be a bear.

b) Have worse concurrent performance than using a properly designed datastructure - e.g. a ConcurrentHashMap has finer grained locking, so it actually is possible for multiple threads to write to it in a safe manner.

Martin Fowler makes strong points for not designing for performance up front. There's no way for anyone, you or me, to predict what the performance will be in the class. Also, if it slows the entire app down by .01%, why would you care?

c) Have really ugly code with synchronization logic spread all over the place. This is not a minor point - if your threading code is simple, it's much easier to determine if it's correct

I don't think that's a symptom of the sync block. You can have ugly, almost unreadable code even with the simplest functions. I've seen it. Synchronization just doesn't have anything to do with readable code. Your coding policies should handle that. Multi-threading is probably the most difficult aspect of java development, and I agree we haven't found our utopia. But I can say from my experiences that test driven design goes a long way towards easing the woes of synchronization. I need to start writing my own articles at some point so people pick them apart as well. :)

Martin Fowler makes strong points for not designing for performance up front. There's no way for anyone, you or me, to predict what the performance will be in the class. Also, if it slows the entire app down by .01%, why would you care?

The problem with standard Java synchronization is not performance (it's very fast) but contention. It can become a bottleneck. I think standard synchronization is fine as long as you don't have a lot of threads checking the same monitor often but there are cases where this kind of bottleneck can cause contention issues at a very high level. My personal approach is to first attempt to avoid sharing objects between threads. When I get to the unavoidable minimum amount of communication, only then do I start considering the best approach for managing concurrency. Too often I think Java developers just synchronize mindlessly if they do it at all.

Too often I think Java developers just synchronize mindlessly if they do it at all.

Yea that's a good point too. I was moving more towards that point... and it's not just about synchronizing too. Good code is good, bad code is *BAD*. lol Still though these are things you just have to know when developing multi-threaded applications, and careful use of sync is one of them. It's not a good idea to blame the tool, synchronize, in this case though and say across the board you want to build thread safe code without sync unless you absolutely need it. Many times that approach leads to chasing red herrings, and clients have to pay the bill for that effort. Just a thought...

I agree that even the smallest synchronize block causes performance issues. For example, I think they may have fixed this in Java 6+, but serialization used to have a simple synchronized map to hold class metadata that's used in serialization. This bottleneck showed up in benchmarks I did a few years ago at around 100-200 threads. Replacing the syncrhonized map with a concurrent hash map did wonders... Leif wrote:

But I can say from my experiences that test driven design goes a long way towards easing the woes of synchronization.

TDD, in my experience does nothing towards easing synchronization woes as these problems only show up in benchmarking, if you're lucky, or worse, in production. -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com