15
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.

ROMANIA_engineer
  • 54,432
  • 29
  • 203
  • 199
Trying
  • 14,004
  • 9
  • 70
  • 110
  • 2
    You would not use `volatile` in a singleton. *By definition* the private instance inside the singleton isn't going to change which means there is no danger of a thread caching an old value. This is ignoring you currently have a non-threadsafe implementation because your `getInstance` isn't synchronized. Use an `enum` to create singletons in Java. – Brian Roach Feb 25 '13 at 22:44
  • http://jeremymanson.blogspot.com/2008/11/what-volatile-means-in-java.html – happybuddha Feb 25 '13 at 22:45
  • 1
    For almost all uses you might as well just use the initialisation expression as class loading is done lazily. If you're using the class for some other reason perhaps without using this instance (sounding bad), then you a nested class to contain the static will do the job nicely. – Tom Hawtin - tackline Feb 25 '13 at 22:52
  • If you absolutely have to use a singleton, consider this: http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom – dnault Feb 25 '13 at 22:54

9 Answers9

19

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;
      }
 }
corsiKa
  • 81,495
  • 25
  • 153
  • 204
  • I'd rather use lock objects http://docs.oracle.com/javase/tutorial/essential/concurrency/newlocks.html – ssedano Feb 25 '13 at 22:47
  • 1
    @corsiKa Thanks for ur prompt reply. Can i put it this way "As volatile only solves visibility problem by conforming to happen-before idiom but here in this case we need visibility as well as atomicity i.e. checking the condition and initializing the object(resource). So either synchronized or static initializer is required". Please correct me if i am wrong. – Trying Feb 25 '13 at 22:47
  • @user2109070 I would say that's a sufficient summary. If I were doing a code review on the code, that would be a reasonable statement to make. – corsiKa Feb 25 '13 at 22:49
  • @ssedano Locks aren't a bad idea. I'm not convinced a lock is necessary here, as the lock is only ever single entry, single exit. I suppose it would reduce the code footprint. – corsiKa Feb 25 '13 at 22:50
  • @corsiKa I added it just for the record. It is true that your solution is perfectly valid. – ssedano Feb 25 '13 at 22:52
  • 4
    Or you could use an `enum` and avoid all of this. http://stackoverflow.com/questions/70689/what-is-an-efficient-way-to-implement-a-singleton-pattern-in-java – Brian Roach Feb 25 '13 at 22:54
  • @Brian not exactly - enum isn't going to effectively handle lazy-loaded resources. The synchronization problem will still exist. – corsiKa Feb 25 '13 at 23:22
  • Works just fine. Nothing is initialized until you touch the instance; you get lazy loading for free as well. – Brian Roach Feb 26 '13 at 00:13
  • 1
    @ssedano It's probably easier to intrinsic locks. If you don't need the supported methods (like tryLock or multiple conditions) you should probably use synchronized. – John Vint Feb 26 '13 at 21:22
  • @BBrian perhaps. I would be concerned that something may touch the class for unrelated purposes and perhaps prematurely trigger the loading before the proper resources were ready. I agree that using an enum for singleton is preferred for other reasons, you could probably incorporate it into the solution and get a lot of features for free. – corsiKa Mar 01 '13 at 20:59
16

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.

7

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();
    }
}
John Vint
  • 39,695
  • 7
  • 78
  • 108
1

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

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

masoud
  • 55,379
  • 16
  • 141
  • 208
0

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.

ssedano
  • 8,322
  • 9
  • 60
  • 98
0

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.

0

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?.

Community
  • 1
  • 1
Jason Sankey
  • 2,328
  • 1
  • 15
  • 12
0

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.

Bytekoder
  • 192
  • 1
  • 7
  • 23
0

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;
    }
}
Oleg Imanilov
  • 2,591
  • 1
  • 13
  • 26