views:

1425

answers:

11

If I create classes, that are used at the moment only in a single thread, should I make them thread-safe, even if I don't need that at the moment? It could be happen, that I later use this class in multiple threads, and at that time I could get race conditions and may have a hard time to find them if I didn't made the class thread-safe in the first place. Or should I make the class not thread-safe, for better performance? But premature optimization is evil.

Differently asked: Should I make my classes thread-safe if needed (if used in multiple threads, otherwise not) or should I optimize this issue then needed (if I see that the synchronization eats up an important part of processing time)?

If I choose one of the both ways, are there methods to reduce the disadvantages? Or exists a third possibility, that I should use?

EDIT: I give the reason this question came up to my mind. At our company we have written a very simple user-management that writes the data into property-files. I used it in a web-app and after some work on it I got strange errors, that the user-management forgot about properties of users(including name and password) and roles. That was very annoying but not consistently reproducible, so I think it was race condition. Since I synchronized all methods reading and writing from/on disk, the problem disappeared. So I thought, that I probably could have been avoided all the hassle, if we had written the class with synchronization in the first place?

EDIT 2: As I look over the tips of Pragmatic Programmer, I saw tip #41: Always Design for Concurrency. This doesn't say that all code should be thread-safe, but it says the design should have the concurrency in mind.

A: 

If you want to follow what Sun did in the Java API, you can take a look at the collection classes. Many common collection classes are not thread-safe, but have thread-safe counterparts. According to Jon Skeet (see comments), many of the Java classes were originally thread-safe, but they were not benefiting developers, so some classes now have two versions - one being thread-safe and the other not thread-safe.

My advice is to not make the code thread-safe until you have to, as there is some overhead involved with thread-safety. I guess this falls into the same category as optimization - don't do it before you have to.

Thomas Owens
Actually, Sun's approach was the other way round - the original collection classes (and StringBuffer) were thread-safe, then they realised it wasn't actually helping anyone, so when they redesigned the collection classes they made them non-thread-safe.
Jon Skeet
Really? I will correct my posting to make sure the history is right. But I know that Sun did it for performance reasons, which was my point. Thanks.
Thomas Owens
A: 

Design separately the classes to use from multiple threads and document other ones to be used from only single thread.

Single threaded ones are much easier to work with.

Separating the multithreaded logic helps to make the synchronization correct.

iny
+17  A: 

I used to try to make everything thread-safe - then I realised that the very meaning of "thread-safe" depends on the usage. You often just can't predict that usage, and the caller will have to take action anyway to use it in a thread-safe way.

These days I write almost everything assuming single threading, and put threading knowledge in the select few places where it matters.

Having said that, I do also (where appropriate) create immutable types, which are naturally amenable to multi-threading - as well as being easier to reason about in general.

Jon Skeet
Do you have an example, where the usage can break thread-safety of a class?
Mnementh
Very easily - take a collection where every single operation is thread-safe in itself. Now iterate over it - bang, thread-safety is gone. You don't want each individual operation to take out a lock - you need a lock for the whole time you're iterating. The colleciton itself can't do that for you.
Jon Skeet
OK, with this explanation your post is worth an upvote. Thanks.
Mnementh
Also, some naive thread-safe implementations don't take the memory model into the account, so you end up with code that is thread-safe when run on a single processor but then isn't thread-safe when threads reside on different processors.
Alexander
Yep, and the vm can create threads that you do not know about plus jit can come into operation some thousands of iterations into run-time causing event scenario that are not disclosed in testing. Probably the canonical example is when recently I wrote classes based on caveats in the java.util It wasn't doing what it said, I had to implement full-copy semantics on top of deciphering an Exception that did not even have "null" - it was using the name of the class that generated the original and balking because it was a system thread that held the reference.
Nicholas Jordan
+5  A: 

Follow the prinicple of "as simple as possible, but no simpler." Absent a requirement, you should not make them thread-safe. Doing so would be speculative, and likely unnecessary. Thread-safe programming adds much more complexity to your classes, and will likely make them less performant due to synchronization tasks.

Unless explicitly stated that an object is thread-safe, the expectation is that it is not.

dpurrington
+1  A: 

You should absolutely know which segments of your code will be multi-threaded and which won't.

Without being able to concentrate the area of multithreadedness into a small, controllable section, you will not succeed. The parts of your app that are multi-threaded need to be gone over carefully, fully analyzed, understood and adapted for a multi-threaded environment.

The rest does not and therefore making it thread-safe would be a waste.

For instance, with the swing GUI, Sun just decided that none of it would be multi-threaded.

Oh, and if someone uses your classes--it's up to them to ensure that if it's in a threaded section then make it threadsafe.

Sun initially came out with threadsafe collections (only). the problem is, threadsafe cannot be made un-threadsafe (for performance purposes). So now they came out with un-threadsafe versions with wrappers to make them threadsafe. For most cases, the wrappers are unnecessary--assume that unless you are creating the threads yourself, that your class does not have to be threadsafe--but DOCUMENT it in the javadocs.

Bill K
A: 

"Always" is a very dangerous word in software development... choices like this are "always" situational.

dacracot
+3  A: 

I personally would only design classes that are "thread-safe" when needed - on the principle of optimise only when needed. Sun seem to have gone the same way with the example of single threaded collections classes.

However there are some good principles that will help you either way if you decide to change:

  1. Most important: THINK BEFORE YOU SYNCHRONIZE. I had a colleague once who used to synchronize stuff "just in case - after all synchronized must be better, right?" This is WRONG, and was a cause of multiple deadlock bugs.
  2. If your Objects can be immutable, make them immutable. This will not only help with threading, will help them be safely used in sets, as keys for Maps etc
  3. Keep your Objects as simple as possible. Each one should ideally only do one job. If you ever find you might want to synchronise access to half the members, then you possibly should split the Object in two.
  4. Learn java.util.concurrent and use it whenever possible. Their code will be better, faster and safer than yours (or mine) in 99% of cases.
  5. Read Concurrent Programming in Java, it's great!
Nick Fortescue
@Nick: I was interested to see your comment that synchronisation caused multiple deadlock bugs, would you consider doing a jeopardy style question where you ask how can a synchronized piece of code cause deadlocks and provide an answer ? It would be very informative to hear about this.
_ande_turner_
@Nick: Since a thread cannot deadlock against itself, the application that deadlocked must have multiple threads accessing the same objects. Therefore some synchronization was definitely required.
Stephen C
You are right, I'm not saying synchronization is bad - it obviously isn't. I'm saying Synchronization without thinking is bad.
Nick Fortescue
+2  A: 

I found the JCIP annotations very useful to declare which classes are thread-safe. My team annotates our classes as @ThreadSafe, @NotThreadSafe or @Immutable. This is much clearer than having to read Javadoc, and FindBugs helps us find violations of the @Immutable and @GuardedBy contracts too.

Chris Dolan
+8  A: 

Start from the data. Decide which data is explicitly shared and protect it. If at all possible, encapsulate the locking with the data. Use pre-existing thread-safe concurrent collections.

Whenever possible, use immutable objects. Make attributes final, set their values in the constructors. If you need to "change" the data consider returning a new instance. Immutable objects don't need locking.

For objects that are not shared or thread-confined, do not spend time making them thread-safe.

Document the expectations in the code. The JCIP annotations are the best pre-defined choice available.

Alex Miller
I like this answer. You decide for one way (don't synchronize too early), but you also provide some solutions, that will help to reduce the danger getting into problems.
Mnementh
+1  A: 

Just as a side remark: Synchronization != Thread-safety. Even so you might not concurrently modify data, but you might read it concurrently. So keep the Java Memory Model in mind where synchronization means making data reliable available in all threads, not only protecting the concurrent modification of it.

And yes, in my opinion thread-safety has to built in right from the beginning and it depends on the application logic if you need handling of concurrency. Never assume anything and even if your test seems to be fine, race conditions are sleeping dogs.

ReneS
A: 

To avoid race conditions, lock on only one object - read descriptions of race conditions tediously and you will discover that cross-locks ( race condition is a misnomer - race comes to halt there ) are always a consequence of two + threads trying to lock on two + objects.

Make all methods synchronized and do testing - for any real world app that actually has to deal with the issues sync is a small cost. What they don't tell you is that the whole thing does lockout on 16 bit pointer tables ... at that point you are uh,...

Just keep your burger flippin resume' current.

Nicholas Jordan