Making Singleton pattern thread safe

[Rohit Kejriwal]

Hi,
Recently I was asked to make all the use of singleton in our existing java application thread safe. I know Singleton is not thread safe and can lead to the creation of multiple instance if two or more thread accesses the getInstance() block at the same time. But I had some doubt for which I will need your opinion.
1. If my application is not using thread at all in the application, is it worth make the singleton usage thread safe?
2. There are singleton classes that has no variables but just few public method, is it worth making these classes thread safe.
3. Is there anything that I need to know before making singleton thread safe?
Thanks in advance for your help.

[Campbell Ritchie]

What makes you think that a Singleton will be the solution to whichever problem you are facing?
Have a look at this thread about a similar topic.

[Steve Luke]

Rohit Kejriwal wrote:Hi,
Recently I was asked to make all the use of singleton in our existing java application thread safe. I know Singleton is not thread safe and can lead to the creation of multiple instance if two or more thread accesses the getInstance() block at the same time.

Only if you make the getInstance() method more complicated than it has to be.

public class ASingleton
{
    private static final ASingleton INSTANCE = new ASingleton();

    public static ASingleton getInstance()
    {
        return INSTANCE;
    }
}

Simple, will only return one instance, and all the synchronization is handled by the JVM during the class load process. That doesn't make the rest of the class thread safe, but it does make the creation and access of the single instance safe (unless you look for creative ways to break it).

Rohit Kejriwal wrote: But I had some doubt for which I will need your opinion.
1. If my application is not using thread at all in the application, is it worth make the singleton usage thread safe?

So it doesn't have a GUI? If there truly is only the main thread in the application, then there is no need to add thread safety. It is just really not very common to have only the one thread.

Rohit Kejriwal wrote:2. There are singleton classes that has no variables but just few public method, is it worth making these classes thread safe.

A singleton with no variables is a singleton with no state. A class with no state should not require an instance, so it should not be a singleton. It should use only static methods, which would make it a Utility class. You should drop the requirement for an instance (or many instances...). Or, you should make it not a singleton and allow as many instances of the class as client code wants, since there is no side effect to doing so. The second option lends itself better to certain designs and testing. But both seem to be suitable choices, and I would lean towards the static Utility class. Either way, no Singleton.

Rohit Kejriwal wrote: 3. Is there anything that I need to know before making singleton thread safe?
Thanks in advance for your help.

There is a lot you need to know. Making things thread safe is no joke. The simple solution is to lock everything, but that leads to trouble all its own (not just efficiency problems, but also deadlocks.) So read up on the threading - there are books on it (Java Concurrency in Practice as an example) which are well worth the read, and much better info than what you can get in a thread on this forum (books can be much more thorough than forum posts.)

[Campbell Ritchie]

As I said yesterday, why do you want to use singletons at all?

[Rohit Kejriwal]

Thanks Steve, your reply makes a lot of sense to me.

Ritchie, The application I am working on was developed long back so I have very little idea why Singleton was used. There are classes in application were we do need singleton to load static data at startup Which I guess is good but there are certain helper and mapper classes which has no state but still Singleton has been used for these classes. Recently PMD was run against the application and it came up with the Singleton not thread safe violation. And I was asked to work on this. I am currenntly trying to figure out ways to handle these.

Thanks.

[Winston Gutkowski]

Rohit Kejriwal wrote:Recently I was asked to make all the use of singleton in our existing java application thread safe.

Like the others, I don't like those instructions, because somebody's telling you how to do this, rather than letting you decide.

But, in answer to your specific questions:

1. If my application is not using thread at all in the application, is it worth make the singleton usage thread safe?

If what you said is guaranteed...ALWAYS - No.

2. There are singleton classes that has no variables but just few public method, is it worth making these classes thread safe.

No.

3. Is there anything that I need to know before making singleton thread safe?

I think the guys have covered that one for you: make it immutable. And the easiest way to do that is by direct assignment of the instance.

Winston

[Jeff Verdegan]

Steve Luke wrote:A class with no state should not require an instance, so it should not be a singleton. It should use only static methods, which would make it a Utility class. You should drop the requirement for an instance (or many instances...).

Just to nitpick a bit, that's not necessarily true. You may want to define some polymorphic behavior, which doesn't in itself require state, and which you can't get with static methods.

[Jeff Verdegan]

Winston Gutkowski wrote:

Rohit Kejriwal wrote:If my application is not using thread at all in the application, is it worth make the singleton usage thread safe?

If what you said is guaranteed...ALWAYS - No.

That means that you have to also be certain that any 3rd-party library that you're using and that may get access to your singleton doesn't to any multithreading internally, or that it properly handles any required syncing itself. For instance, even a simple logging library might use multithreading to provide asynchronous logging for better performance.

Singleton s = ...;
s.setState("X");
logger.log(s);
s.setState("Y");
logger.log(s);

In the above code, we might expect the output to be:

X
Y

but we could get "Y" on both lines.

We see only a single thread, that sets the state to X, then logs it, then sets it to Y, then logs it again. However, if the log() call in the current thread is just queueing up requests that will be pulled from the queue and written out by a separate thread, the singleton's state could change to Y while the first log request is still in the logger's queue.

(That probably wouldn't happen. I would expect that the log() call would generate the String synchronously and put that on the queue. This is just a simple example to illustrate a point.)

And just to be picky, every Java application will have at least one thread executing, and the vast majority will have several, even if you don't explicitly write any multithreaded code yourself.

[Junilu Lacar]

Rohit Kejriwal wrote:Recently PMD was run against the application and it came up with the Singleton not thread safe violation. And I was asked to work on this. I am currenntly trying to figure out ways to handle these.

PMD probably detected one of the broken methods for lazy initialization of a singleton. I suggest you read through the Wikipedia article on singleton pattern and implement one of the recommended fixes. Read the article carefully because it contains code samples of broken implementations as well. If you're going to copy something from there, make sure it's not an example of a bad implementation.

[Pat Farrell]

Junilu Lacar wrote:I suggest you read through the Wikipedia article on singleton pattern and implement one of the recommended fixes.

While the idea of copying good code over bad is correct, I strongly suggest you not use a singleton. They cause far more problems than they solve. They make proper unit testing impossible. They are simply a global pollution of the namespace.

[Junilu Lacar]

While all these admonitions against the use of Singleton are certainly well-intentioned we should keep in mind that the OP may not have much of a choice, at least in the short term, but to fix the broken singleton implementation(s) reported by the static analysis tool.

@OP: seriously consider the advice of avoiding Singleton if you can but if your hands are tied then I reiterate my recommendation to find a proper implementation that will mitigate the problem reported by PMD: read the wikipedia article very carefully and use one of the recommended approaches.

[Jeff Verdegan]

Junilu Lacar wrote:

Rohit Kejriwal wrote:Recently PMD was run against the application and it came up with the Singleton not thread safe violation. And I was asked to work on this. I am currenntly trying to figure out ways to handle these.

PMD probably detected one of the broken methods for lazy initialization of a singleton. I suggest you read through the Wikipedia article on singleton pattern and implement one of the recommended fixes.

The best fix (short of getting rid of the singleton completely) is to not use lazy instantiation at all. There's no need for it, and it just leads to unnecessary headaches.

[Rohit Kejriwal]

Thanks everyone for your post.

Ok, so my application infact has thread (initially I had ignored the fact that the multiple access from UI is treated as thread). But again from all the discussion and post above one thing is sure that it make no sense to use Singleton if it's a utility class or the class do not maintain any state. But I am not able to figure out what will be the best option- making methods Static or allowing multiple instances. How will these effect the performance of the system. This application is a huge e-commerce application with more then 40,000 transaction daily. So application performance will be one of the main criteria to decide on the actual implementation. Please share your thoughts on this.

[Steve Luke]

The affect on performance is unpredictable. You should design based on what makes sense. Then measure performance and if it is not up to snuff, figure out where the performance problem is (using a profiler, not by guessing) and fix that. But the choice between static, Singleton or multiple instances, should not be a performance choice, but a design choice.

[Pat Farrell]

Static methods that do not keep state are trivial to implement. Just pass in the arguments, use local variables and return the result.

You can implement a factory class/method that simply returns the one and only instance of something. With this, you can change the factory for unit testing to deliver a mock object and all of your code just uses it as normal, performing unit tests. Say you used to have a FoobazSingleton. Refector your code to have a Foobaz interface and the factory:

class FoobazFactory {

public static Foobaz makeFoobaz() {
  Foobaz rval = whatever....
  return rval;
}
}

Remember, premature optimization is the root of all evil. 40,000 transactions a day is really not a lot. If your "day" is 10 hours, then you are doing 4,000 transactions per hour. There are 3600 seconds in an hour, so you are only doing a bit more than one transaction per second. The universal rule of improving performance is to write the code and then measure where it is really spending its time. Fix the slow parts that you measure. Do not waste time "fixing" things you think are slow until you have measured real performance.

[Winston Gutkowski]

Rohit Kejriwal wrote:But I am not able to figure out what will be the best option- making methods Static or allowing multiple instances.

Like Steve said, I doubt whether those your only two choices; and you shouldn't base your decision on what's fastest, you should base it on what's right.

Just off the top of my head, I generally lean against making methods static if I can avoid it, because it's a major PITA to refactor if you get it wrong.

Winston

[Junilu Lacar]

Rohit Kejriwal wrote: making methods Static or allowing multiple instances. How will these effect the performance of the system. This application is a huge e-commerce application with more then 40,000 transaction daily. So application performance will be one of the main criteria to decide on the actual implementation.

First of all, don't tune your application based on speculation and guessing. Use a profiling tool. Also, scaling and performance issues are usually best solved with hardware. I doubt static methods vs. multiple instances of classes will have a very significant impact on the scale that you're talking about. Look into clusters and load balancers. Look over these search results: scaling your java ee applications