Is this singleton implementation correct?

Question

As the double lock checking doesn't work on optimized compilers I am removing it from my singleton class and instead of going for early initialization.

Below is my new singleton class :

class MySingleton {
    private static volatile MySingleton myInstance = new MySingleton();
    public static getInstance() {
        return myInstance;
    }
}

Apart from getInstance(), there are setter methods in the class which set the values of the member fields.

Hope this implementation will not cause any data inconsistency when multiple threads will be updating various member fields using the same object.

Any suggestions or inputs are welcome.

@HadiJ I completely agree i need to define a private constructor. But i suppose this would be thread safe and there is no need for synchronization as the static field would be created during class load. Since my reference is static it will always be available to any thread that tries to access. — ANKIT SRIVASTAVA
– ANKIT SRIVASTAVA, Commented Nov 26, 2019 at 10:04

Stephen C · Accepted Answer · 2019-11-27 07:37:43Z

3

This code is thread-safe. Indeed, you can remove the volatile and it will still be thread-safe.

The myInstance variable is initialized when static initialization is triggered (if it hasn't already happened) by the first getInstance call. There is a happens-before from the initialization and the first call.

However, you also said this:

Hope this implementation will not cause any data inconsistency when multiple threads will be updating various member fields using the same object.

You haven't shown any of the code for doing that, so we can't tell you whether that code is thread-safe. If MySingleton instances are mutable, then the state-related methods must synchronize appropriately to prevents memory hazards and race conditions. The code that you have shown us does not address this.

Since the default MySingleton constructor is public, this is not a proper singleton class. Multiple instances can be created.

This implementation differs from many other implementations in that the distinguished MySingleton instance is created eagerly rather than lazily. This simplifies the problem considerably.

Note that lazy initialization of singletons is often unnecessary. If you don't specifically need lazy initialization, then the simpler eager alternative is just fine.

edited Nov 27, 2019 at 7:37

answered Nov 26, 2019 at 14:36

Stephen C

723k95 gold badges849 silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Holger Over a year ago

As you said, the instance is created on the first getInstance call, which is all but "created eagerly". The "many implementations" doing it differently usually try to solve a nonexistent problem.There's a mismatch between the appearances in media and the actual relevance of the singleton pattern in real life anyway.

Holger Over a year ago

And it must be emphasized that the implementation of the singleton has no impact on the safety of the updates made by the setter methods the OP mentioned in the question.

Stephen C Over a year ago

Incorporated. Thanks.

Matt Timmermans · Accepted Answer · 2019-11-27 03:48:24Z

0

Your singleton is thread-safe, but there are things other than a call to getInstance() which could cause the MySingleton class to be initialized, which would construct the instance.

The usual way around this is to put the singleton in a private nested class like this:

class MySingleton {
    private MySingleton() {
        // there can only be one
    }
    private static class Holder {
        static final MySingleton instance = new MySingleton();
    }
    public static MySingleton getInstance() {
        return Holder.instance;
    }
}

Now the instance can only be constructed when the Holder class is initialized, and that only happens in the first call to getInstance()

edited Nov 27, 2019 at 3:48

answered Nov 27, 2019 at 3:39

Matt Timmermans

60.9k3 gold badges57 silver badges106 bronze badges

Collectives™ on Stack Overflow

Is this singleton implementation correct?

2 Answers 2

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related