0

Why do we need to add volatile to field to prevent invalid data retrieval? Can't we do the same thing by adding synchronized to method declaration instead of to a block of code?

public class LazySingleton {

private static volatile LazySingleton instance;

private LazySingleton() {
}

public static LazySingleton getInstance() {
    if (instance == null) {
        synchronized (LazySingleton.class) {
            if (instance == null) {
                instance = new LazySingleton();
            }
        }
    }
    return instance;
}
}

and

public class LazySingleton {

private static LazySingleton instance;

private LazySingleton() {
}

public static synchronized LazySingleton getInstance() {
    if (instance == null) {
        instance = new LazySingleton();
    }
    return instance;
}
}
kaka
  • 597
  • 3
  • 5
  • 16
  • Because with the first example, after the value has been initialized, you can access it without any synchronization at all. – Jorn Jul 20 '23 at 14:06
  • Why are you using the singleton pattern at all? The singleton pattern is terrible for testing (makes it hard to mock dependencies) and generally a bad idea since it tightly couples your code. Have you considered dependency injection / inversion of control instead? – lance-java Jul 20 '23 at 14:41
  • Singletons are best avoided; but if you must, implement them as a single-element enum; or, if you can't do that, using a lazy holder. The fact you're asking this question shows how hard an actual singleton is to implement correctly. – Andy Turner Jul 20 '23 at 14:44

2 Answers2

1

You don't need double locking. AT ALL.

The idea is generally 'hey, it is a waste of time to create that singleton unless code actually needs that singleton, so lets make it only the first time somebody needs it'.

A fair consideration, though, of course, in practice, the vast majority of singletons do no relevant initialization process whatsoever (their constructors are empty or trivial). Even if not, java already applies this principle for classes itself. You may think that java, upon starting, loads every class on the classpath first, and only then starts executing your app by invoking your main method. Not so. Instead, java loads nothing, and starts by executing your main method. As part of doing this, your main class will have to be loaded. And as part of doing that, everything your main class 'touches' is soft-loaded.

Touched by your main class means e.g. one of the fields in your main class has String as a type, or of course String which shows up in the signature of the main method - that causes the java classloader system to pause its attempting to actually execute your main method and go load String instead.

The process ends there though - in basis just because you mention a type someplace in Main does not mean java will then fully load that type (i.e. load everything that touches, like tracing a path across a spiderweb that soon colours in the entire web). Instead those types are only 'fully loaded' once you execute code that actually uses that type.

And, of course, once some type has been loaded, java ensures it is never loaded a second time, and, java ensures any given class is never initialized twice, even if 2 threads try to use a class for the first time simultaneously. e.g. if you do:

class Test {
  private static final String foo = hello();

  private static String hello() {
    System.out.println("Hello!");
    return "";
  }
}

Any execution of any java program with the above code in it has only three options:

  • Hello! is never printed - if this class is never 'needed' to be loaded.

  • It is printed exactly once, ever, for the lifetime of that JVM.

  • More than once, but only if you add more classloaders which effectively means clones of this class end up being loaded which is an academic corner case that doesn't count (had you used double locking it would also initialize more than once in this case).

  • This is simpler and faster than anything you could possibly do here.

Hence, do not lazy load singletons. Yes, a billion tutorials mention it. They're all wrong. It happens. There are entire wikipedia articles about commonly believed falsehoods. Objective proof (such as the above) showing a belief is false should be heeded even if the belief is widespread.

Thus, we get to the right answer for 99.5% of all singletons:


class MySingleton {
  private static final MySingleton INSTANCE = new MySingleton();

  public static MySingleton of() {
    return INSTANCE;
  }
}

Simple. Effective. and already lazy-loading because java's class loader system itself is lazy loading.

The one exception to this rule, which should come up almost never, is if it is reasonable to expect that code will 'touch' a class (mention the class someplace, e.g. use ThatSingleton.class as an expression or have a method that has a parameter of type ThatSingleton which nobody is likely to ever call), but never actually want an instance of the singleton. That is an extremely weird situation to be in - normally if anybody so much as mentions the singleton type, that code will immediately invoke the 'get me that singleton' method. After all, the primary reason to mention a type but not call the 'get me the singleton' method is if you want to invoke static methods on it. Who makes singletons with static methods? That's... silly.

The exception applies to me!

I doubt it.

But if it does, we can still rely on the best, safest, fastest 'init only once' mechanism java offers, which is the classloader:

public class MySingleton {
  public static MySingleton of() {
    class MySingletonHolder {
      static final MySingleton INSTANCE = new MySingleton();
    }
    return MySingletonHolder.INSTANCE;
  }
}

EDIT: The above snippet is a nice suggestion by /u/Holger, but it requires a relatively new java version as sticking static things in inner classes used to be invalid java code. If compiling the above code tells you that 'you cannot put static elements in a non-static class' or similar, simply move the Holder class outside the method and mark it private static final.

This is objectively and in an absolute sense (As in, no exceptions, period) the best option - even in that exceedingly exotic case as described before. new MySingleton() will never be executed unless somebody invokes of(), in which case it will be executed precisely once. No matter how many threads are doing it simultaneously.

Explanatory: What's wrong with your snippets

Your question inherently implies that you think synchronized is either simpler than volatile, faster than volatile, or both. This simply is not true: volatile is usually faster, which is why the volatile example shows up more often as 'the best way to do it' (it is not - using the classloader is better, see above).

Explanatory: What is the hubbub about?

The general problem with your synchronized example is that any call to of() will have to acquire a lock and then release it immediately after checking that, indeed, INSTANCE is already set. Just the synchronized part (the acquiring of the lock) is, itself, quite slow (thousands, nay, millions of times slower than new MySingleton() unless your singleton constructor does something very very serious, such as parse through a file). You pay this cost even days after your app has started - after all, every call to of(), even the millionth, still has to go through synchronized.

The various tricks such as double-locking etc try to mitigate that cost - to stop paying it once we've gotten well past initializing things. The simple double-locking doesn't actually 'work' (is not guaranteed to only ever make 1 instance), with volatile it does and now you're paying the lesser, but still significant cost of reading a volatile field every time anybody calls the 'get me the singleton instance' method. This also is potentially quite slow.

All java code has to go through the initialisation gate anytime it touches any class. Which is why the classloader system has very efficient processes for this. Given that you already have to pass that gate, might as well piggyback your lazy initialization needs off of it - which gets us back to: Use my first snippet. Unless you have written something completely ridiculous such as a singleton that also has static methods, in which case, use my second snippet. And know that all blogs, tutorials, examples, etc that show any sort of double locking anything are all far worse.

rzwitserloot
  • 85,357
  • 5
  • 51
  • 72
  • 1
    While you are correct that just using class initialization is the better choice, I think, you are heavily overestimating the costs of synchronization. As long as there is no heavy contention, it’s not so much worse than a `volatile` variable access. By the way, in recent Java versions, you can hide the `MySingletonHolder` class inside the accessor method: https://jdoodle.com/ia/Kj2 – Holger Jul 20 '23 at 16:23
  • @Holger I didn't use absolute terms, and I was talking about the more general principle of why _others_ focus so much on the difference between `volatile` and `synchronized` - as that is what the question is superficially about. This 'use the class initializers' choice is also far more readable (no need to document the details about how double-checking locking fails due to failing to block re-ordering). Good point about using a method-local class in modern java versions! I shall update this answer, thank you. – rzwitserloot Jul 20 '23 at 16:31
0

Why do we need to add volatile to field to prevent invalid data retrieval?

If your LazySingleton.instance is not volatile, then the first

    if (instance == null) {

test in your first example is not properly synchronized, and therefore exhibits undefined behavior.

Volatile or not, that example is a variation on double-checked locking. You will find much commentary about that, because at one time it was promoted in some quarters, but many implementations in Java and C++ are broken. In particular, removing the volatile from your first example breaks it.

Can't we do the same thing by adding synchronized to method declaration instead of to a block of code?

Yes, in the sense that your second code is correctly synchronized and does have the property that the singleton is instantiated only once, on the first request for it. The details of how that is achieved are not the same, so they might not perform identically.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157