Trying
Trying

Reputation: 14278

singleton with volatile in java

class MyClass
{
      private static volatile Resource resource;

      public static Resource getInstance()
      {
            if(resource == null)
                  resource = new Resource();
            return resource;
      }
 }

Here my doubt is according to java concurrency in practice if you use volatile, safe publication happens (i.e. as soon the reference is visible to another thread the data is also available). So can I use it here? But if it is correct then suppose thread1 now checks "resource" and it's null so it starts creating the object. While thread1 is creating the objet another thread i.e. thread2 comes and start checking the value of "resource" and thread2 finds it as null (assume creating "resource" object takes some considerable amount of time and as thread1 has not yet completed the creation so the safe publication hasn't happened hence unavailable to thread2 )then will it also start creating the object? if yes then class invariant breaks. Am I correct? Please help me in understanding this specially use of volatile here.

Upvotes: 15

Views: 17913

Answers (9)

Oleg Imanilov
Oleg Imanilov

Reputation: 2751

Here is my suggestion to add volatile & synchronized together.

Note: we still have to do double check.

public class MySingleton {
    private static volatile MySingleton instance;
    private MySingleton() {}

    synchronized private static void newInstance() {
        if(instance == null) {
            instance = new MySingleton();
        }
    }

    public static MySingleton get() {
        if(instance == null) {
            newInstance();
        }
        return instance;
    }
}

Upvotes: 0

user2435300
user2435300

Reputation:

volatile solves one issue that is visibility issue . If you are writing to one variable that is declared volatile then the value will be visible to other thread immediately. As we all know we have different level of cache in os L1, L2, L3 and if we write to a variable in one thread it is not guaranteed to be visible to other, so if we use volatile it writes to direct memory and is visible to others. But volatile does not solve the issue of atomicity i.e. int a; a++; is not safe. AS there are three machine instructions associated to it.

Upvotes: 16

Bytekoder
Bytekoder

Reputation: 302

first of all, having a Singleton this way, you are essentially creating a global object which is a bad practice. I reckon you use Enums instead.

Upvotes: 0

masoud
masoud

Reputation: 56479

I think, you should use syncronized keyword before getInstance definition.

For better performance you can use Double-checked locking pattern:

Upvotes: 1

John Vint
John Vint

Reputation: 40256

I know you aren't asking about better solutions but this is definitely worth if you are looking for a lazy singleton solution.

Use a private static class to load the singleton. The class isn't loaded until invocation and so the reference isn't loaded until the class is. Class loading by implementation is thread-safe and you also incur very little overhead (in case you are doing repetitive volatile loads [which may still be cheap], this resolution always normal loads after initial construction).

class MyClass {
    public static Resource getInstance() {
        return ResourceLoader.RESOURCE;
    }

    private static final class ResourceLoader {
        private static final Resource RESOURCE = new Resource();
    }
}

Upvotes: 7

ssedano
ssedano

Reputation: 8432

volatile keyword guarantees that read and write to that variable are atomic.

According to the tutorial

Reads and writes are atomic for all variables declared volatile

Using volatile variables reduces the risk of memory consistency errors, because any write to a volatile variable establishes a happens-before relationship with subsequent reads of that same variable. This means that changes to a volatile variable are always visible to other threads. What's more, it also means that when a thread reads a volatile variable, it sees not just the latest change to the volatile, but also the side effects of the code that led up the change.

Upvotes: 0

Jason Sankey
Jason Sankey

Reputation: 2338

You are correct, in this case the Resource may be constructed twice due to the race you describe. If you want to implement a singleton (without explicit locking) in Java 5+, use an enum singleton as described in answers to What is an efficient way to implement a singleton pattern in Java?.

Upvotes: 0

user2030471
user2030471

Reputation:

When applied to a field, the Java volatile guarantees that:

  1. (In all versions of Java) There is a global ordering on the reads and writes to a volatile variable. This implies that every thread accessing a volatile field will read its current value before continuing, instead of (potentially) using a cached value. (However, there is no guarantee about the relative ordering of volatile reads and writes with regular reads and writes, meaning that it's generally not a useful threading construct.)

  2. (In Java 5 or later) Volatile reads and writes establish a happens-before relationship, much like acquiring and releasing a mutex.

More info.

Upvotes: 0

corsiKa
corsiKa

Reputation: 82559

You are correct, multiple threads could try to create a Resource object. Volatile just guarantees that if one thread updates the reference, all other threads will see the new reference, not some cached reference. This is slower, but safer.

If you require only a single resource that is lazy loaded, you need to do something like this:

class MyClass
{
      private static volatile Resource resource;
      private static final Object LOCK = new Object();

      public static Resource getInstance()
      {
            if(resource == null) { 
                synchronized(LOCK) { // Add a synch block
                    if(resource == null) { // verify some other synch block didn't
                                           // write a resource yet...
                        resource = new Resource();
                    }
                }
            }
            return resource;
      }
 }

Upvotes: 19

Related Questions