Thread-safety refactoring

Question

I am trying to make a Java application thread-safe. Unfortunately, it was originally designed for a single-user mode and all the key classes are instantiated as singletons. To make matters worse, there is a whole bunch of interfaces working as constants containers and numerous static fields.
What would be considered as a good practice in this case?

• There is a single entry point, so I could synchronize that and just use pooling (sort of), but if the calls take more than a minute on average, all other threads in the queue would have to wait for a long time...
• Since the test code coverage is not quite optimal and I can't be sure if I overlooked something, some hints about bad implementation patterns (similar to those stated above) in this area would be useful.
• I know the best thing would be to rewrite the whole structure, but this isn't an option.

Answer 1

It doesn't sound like there is a quick fix for this. You should probably start by refactoring the existing code to use good design patterns, with an eye for multi-threading it in the future. Implement the multi-threading as a later step, after you've cleaned it up.

Answer 2

As Jonathan mentions it's doesn't sound like there's a quick fix.

You could consider using ThreadLocal in order to provided a dedicated per-thread singleton. Obviously this may or may not be possible depending on the state stored within the singletons, whether this has to be shared / maintained, etc.

Answer 3

@coldphusion, you'll have to read/analyze code. Using an automated tool, if such a tool exists, would be like shooting yourself in the foot.

Plus, not everything has to be thread-safe. If an object will never be accessed from multiple threads, no need to make it thread-safe. If an object is immutable, then it's already thread-safe.

Be ready to tell your boss "It won't take a few hours or a day, even you know it, so stop asking."

I recommend reading Java Concurrency In Practice.

Answer 4

I will add to @nevermind's advice, since he/she made some very practical points.

Be practical about what you need to change to accomplish your task since there is no magic way. Your existing code, well designed or not, may only need small changes depending on how it is used. Of course this also means a complete redesign may also be in order.

There is no way for anyone here to know (unless they wrote the original code ;-)

For example, if you only need to make access to a single object (singleton or not) threadsafe, this is fairly easily accomplished, possibly without any coding impacts on the caller of such an object.

On the other hand, if you need to modify multiple objects at once to keep the integrity of your data/state, then your efforts will be considerably harder.

Answer 5

Singletons are not a bad thing and do not go against thread-safety, as long as they don't store any state. Just look at any J2EE app; lots of singletons, without any state (only references to other stateless singletons). All state is stored in sessions; you could maybe mimic that, but as others have said, there is no way to automagically transform your app; you will have to make some good analysis to determine how you will refactor it to separate all stateless beans from the stateful ones, maybe encapsulate state in some value objects, etc.

Answer 6

If anyone should be also interested on the topic - I found a pretty detailed tutorial on "what (not) to do" - with common mistakes and best practices.
Unfortunately, it's only available in German atm :|