0

How reading (outside critical resource block) and writing (inside critical resource block) does not have atomicity issues.

I have read and discussed with various people but most people don't answer if both operations are atomic and how atomicity is actually achieved for above problem.

class ABC {
    private static volatile ABC abcInstance;
    static ABC getInstance(){
        if(abcInstance == null){
            synchronized(ABC.class){
                if(abcInstance == null){
                    abcInstance = new ABC();
                    return abcInstance;
                }
            }
        }
        return abcInstance;
    }

}

Are if(abcInstance == null) outside synchronisation block and abcInstance = new ABC(); atomic, if not then this way of creating singletons is wrong.

In C++, abcInstance = new ABC(); consists of three instructions broadly speaking:

  1. Create ABC object.
  2. Allocate memory for ABC.
  3. Assign it to abcInstance.

And for optimisations compiler can reorder these three instructions in any way. Suppose it follows 2->3->1 and after instruction 3 interrupt happens and next thread calling getInstance() will read that abcInstance has some value then it will point to something which does not have ABC object.

Please correct me if am wrong for both C++ and Java.

Andy Turner
  • 137,514
  • 11
  • 162
  • 243
Tarun Chawla
  • 508
  • 4
  • 17
  • 1
    Note that your `getInstance()` method should be static. And your `abcInstance` field shouldn't be public. – Andy Turner Sep 11 '19 at 11:46
  • If you want a C++ answer please post some C++ code (or remove the C++ tag). The singleton pattern modern in C++ is much simple and guaranteed by the standard when initialising a block scope static variable. – Richard Critten Sep 11 '19 at 11:50
  • @RichardCritten the modern way to do it in Java [is far simpler too](https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom). – Andy Turner Sep 11 '19 at 11:51
  • "Is if(abcInstance == null) and abcInstance = new ABC(); are atomic" Which `if (abcInstance == null)`? There are two. – Andy Turner Sep 11 '19 at 11:52
  • @AndyTurner [even simpler](https://stackoverflow.com/questions/26285520/implementing-singleton-with-an-enum-in-java) when you take the most modern way. – Kayaman Sep 11 '19 at 11:56
  • Reentrant lazy creation of object should not be part of singleton at all. – user7860670 Sep 11 '19 at 12:03
  • @Kayaman I am wary of trying to put too much into an enum. In particular, enums are meant for things that are constant. Sure, it's a choice though (as is not using a singleton at all!). – Andy Turner Sep 11 '19 at 12:03
  • @AndyTurner yes getInstance() is static pubic and abcInstance should not be public. I just want to understand f(abcInstance == null) outside the synchronisation block and abcInstance = new ABC(); inside the synchronisation block is atomic operation. Am talking about both statements separately. – Tarun Chawla Sep 11 '19 at 14:27
  • @RichardCritten Why to remove C++ tag, problem is kind of common for both languages. For C++ I know that simply static can be used and I am sure for Java same facility would be present but question is about this way of creating singleton which is given as example on various websites. – Tarun Chawla Sep 11 '19 at 14:29
  • @TarunChawla You should also provide a [mcve] for C++ as the code is quite different from Java. If you don't then the question becomes "please write code for me." – Richard Critten Sep 11 '19 at 15:53

2 Answers2

2

This answers the Java part of your question only.

Is if(abcInstance == null) and abcInstance = new ABC(); are atomic, if not then this way of creating singleton is wrong.

It is not atomicity that is the (potential) problem. (Reference assignment is atomic from the perspective of both the thread doing the assignment, and the thread reading the assigned variable.)

The problem is when the value written to abcInstance becomes visible to another thread.

  • Prior to Java 5, the memory model did not provide sufficient guarantees about memory visibility for that implementation to work reliably.

  • In the Java 5 (and later) memory model, there is a happens before relation between one thread's write to a volatile variable and another thread's subsequent read of the variable. This means:

    1. The second thread is guaranteed to see the non-null value of abcInstance if the first thread has written it.
    2. The happens before relation also guarantees that the second thread will see the fully initialized state of the ABC instance create by the first thread.
    3. The synchronized block ensures that only one ABC instance may be created at a time.

This is the authoritative article explaining why old double-checked locking implementations were broken:


As Andrew Turner states, there is a simpler, cleaner way to implement singleton classes in Java: use an enum.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • 1. So this way of initialization to work it is necessary to use volatile object instance. 2. volatile ensures that there is no partial write kind of situation happening. 3. All the websites using non-volatile way of implementing this kind of singleton is simply wrong. is that correct? – Tarun Chawla Sep 11 '19 at 14:41
  • 1) If you are going to use double-checked locking, then `volatile` must be used. And, it is only guaranteed to work with the Java 5+ memory model. 2) I wouldn't put it that way. See what I wrote above. 3) Yes. And it is not just me saying so. This is (in 2019) common knowledge. – Stephen C Sep 11 '19 at 14:46
  • I just wanted to understand if this code has any issues on Java5+ JVM. Is there still any chance where thread 1 which has entered the synchronisation block is not completed but still other thread 2 finds out that instance is nonnull and returns incomplete instance? – Tarun Chawla Sep 11 '19 at 14:51
  • No chance. But don't just trust me. Read and understand the Java Memory Model section of the JLS for yourself. Then analyze your code. – Stephen C Sep 11 '19 at 14:54
1

Here are two typical singleton variants in C++.

First one shared by all threads:

class singleton {
private:
    singleton() {}

public:
    singleton(const singleton&) = delete;

    static singleton& get_instance() {
        static singleton ins;
        return ins;
    }
};

And here's one that that will create one instance per thread that needs it:

class tl_singleton {
private:
    tl_singleton() {}

public:
    tl_singleton(const tl_singleton&) = delete;

    static tl_singleton& get_instance() {
        static thread_local tl_singleton ins;
        return ins;
    }
};
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • That was quick :-) What's wrong with the answer? It was downvoted in < 5 seconds so it should be easy to give me a hint. – Ted Lyngmo Sep 11 '19 at 12:04
  • This approach I was aware, just that my question was specific to this way of creating singleton and map that problem to Java code. As most of Java people could not answer the atomicity part of it but it is easy to discuss the same with C++ people as I wanted to jump into this discussion too. – Tarun Chawla Sep 11 '19 at 14:40
  • @TarunChawla I can't speak for how to do it in Java or if your approach is going to work well there I'm afraid. You _could_ do similar things in C++ (with manual synchronization), but for the creation part of static objects, it's safe to do it as I've shown. Oh, and you've gotten the order wrong when it comes to the C++ part though. Memory is allocated first, then the actual object. – Ted Lyngmo Sep 11 '19 at 15:56
  • @TarunChawla That's nice! ... and to make it clear: I didn't think you did. :-) – Ted Lyngmo Jan 24 '20 at 11:25