28

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.

1
  • Something to help out your analysis is to not think of concurrency as premature optimization, but rather a performance concern akin to BigO. The premature optimization-is-evil observations are in reference to people unrolling loops or cryptic switch-if conditional constructs in C/C++ that are impossible to decipher and maintain, and may not give you any performance gain after the compiler is through with the code (or in java the runtime JIT).
    – reccles
    Nov 10, 2009 at 22:53

13 Answers 13

30

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.

5
  • Do you have an example, where the usage can break thread-safety of a class?
    – Mnementh
    Oct 24, 2008 at 17:29
  • 10
    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
    Oct 24, 2008 at 18:43
  • OK, with this explanation your post is worth an upvote. Thanks.
    – Mnementh
    Oct 25, 2008 at 7:25
  • 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
    Oct 25, 2008 at 10:21
  • 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. Sep 27, 2009 at 22:53
22

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.

1
  • 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
    Oct 25, 2008 at 7:38
6

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.

4

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!
3
  • @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. Nov 15, 2008 at 12:11
  • @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
    Sep 28, 2009 at 2:05
  • You are right, I'm not saying synchronization is bad - it obviously isn't. I'm saying Synchronization without thinking is bad. Oct 2, 2009 at 15:48
3

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.

2

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.

1

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.

1

If I create classes, that are used at the moment only in a single thread, should I make them thread-safe

It is not necessary for a class used by a thread to by itself thread-safe for the program as a whole to be thread-safe. You can safely share objects of non "thread safe" classes between threads if they are protected by appropriate synchronization. So, there is no need to make a class itself thread-safe until that becomes apparent.

However, multi-threading is fundamental (architectural) choice in a program. It is not really something to add as an after thought. So you should know right from the start which classes need to be thread safe.

1

Here's my personal approach:

  • Make objects and data structure immutable wherever you can. That is good practice in general, and is automatically thread safe. Problem solved.
  • If you have to make an object mutable then normally don't bother trying to make it thread safe. The reasoning for this is simple: when you have mutable state then locking / control cannot be safely handled by a single class. Even if you synchronize all the methods, this doesn't guarantee thread safety. And if you add synchronisation to an object that only ever gets used in a single-threaded context, then you've just added unnecessary overhead. So you might as well leave it up to the caller / user to implement whatever locking system is necessary.
  • If you provide a higher level public API then implement whatever locking is required to make your API thread safe. For higher level functionality the overhead of thread safety is pretty trivial, and your users will definitely thank you. An API with complicated concurrency semantics that the users need to work around is not a good API!

This approach has served me well over time: you may need to make the occasional exception but on average it's a very good place to start!

0

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.

2
  • 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
    Oct 24, 2008 at 16:54
  • 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. Oct 24, 2008 at 16:56
0

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.

0

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

0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.