-2

I wrote a below Singleton class. I am not sure whether this is thread safe singleton class or not?

protected static DataTask instance;

protected DataTask() {
    // some code here
}

public static synchronized void setup() {
    if (instance == null) {
        DataTask setupFully = new DataTask();
        instance = setupFully;
    }
}

public static DataTask getInstance() {
    if (instance == null) {
        setup();
    }
    return instance;
}
john
  • 11,311
  • 40
  • 131
  • 251
  • 1
    Prefer enum with single constant as `INSTANCE`, I feel that is the best way. – rajuGT Dec 10 '15 at 20:47
  • You are using the DCL idiom without `volatile` so no, it is not. – Boris the Spider Dec 10 '15 at 20:48
  • Does it always have to be enum. Why cannot we write thread safe class without enum? I don't want to use enum. – john Dec 10 '15 at 20:48
  • @rajuGT that's not lazy. Which might be important to the OP. But I completely agree with you. – Boris the Spider Dec 10 '15 at 20:48
  • @BoristheSpider Can you explain why it is not thread safe with some threads example just for understanding? I thought in my setup method inside if instance null check that will make thread safe? I mean this line `DataTask setupFully = new DataTask(); instance = setupFully;` – john Dec 10 '15 at 20:49
  • `enum` is used because it provides a huge number of guarantees. For example with any other approach, serialization can be used to bypass the single instance. An `enum` guarantees that even when deserialized there will still be only one instance. – Boris the Spider Dec 10 '15 at 20:50
  • 1
    Thread safety is not a 5 minute Stackoverflow question. It requires a lot of careful study. But I assure you, this is not threadsafe. – Boris the Spider Dec 10 '15 at 20:52
  • Why do you use `protected` instead of `private`? Access to constructor should be limited to only singleton class so `protected` can't be used here. – Pshemo Dec 10 '15 at 20:53
  • Re. "careful study" OP please purchase *[Java Concurrency in Practice](http://jcip.net/)* by Brian Goetz. It's the best book there is on Java thread safety. – markspace Dec 10 '15 at 20:53
  • 2
    I feel like Gray has already explained all this in [your last question](http://stackoverflow.com/questions/34184749/how-to-write-getinstance-method-thread-safe). What do you think is missing? Why are you still confused? Explain why you think it'd be thread safe or why you think it won't. – Sotirios Delimanolis Dec 10 '15 at 20:55

1 Answers1

0

This is double-checked locking. It does not work because there is no guarantee in the unsynchronized method when the instance variable is set. This guarantee can be enforced by making instance volatile. There are many articles on double-checked locking including this one.

Community
  • 1
  • 1
Neil Masson
  • 2,609
  • 1
  • 15
  • 23