0
if (searchBox == null) { //1
    synchronized (SearchBox.class) {
        if (searchBox == null) {  //2
            searchBox = new SearchBox();
        }
    }
}

here is my custom class for singleton pattern. in this code, I use double-checked locking as above. As I read many posts on some source, they say that double-check is useful because it prevents two concurrent threads run at same times make two different objects. As per threading concept, at a time an only single thread gets executed by thread scheduler. Then how 2 threads will try to execute the above code.

Please explain to me. What have I understood wrong?

Thanks :)

Kirill Simonov
  • 8,257
  • 3
  • 18
  • 42
saurabh landge
  • 115
  • 1
  • 2
  • 9
  • 2
    There are far better ways to create singleton, such as a single-element enum, or a lazy holder, or even just an eagerly-initialized static final field. Don't use double-checked locking. – Andy Turner Sep 30 '19 at 14:57
  • https://stackoverflow.com/questions/18093735/double-checked-locking-in-singleton check out this. It has been already asked. – Hari Bharathi Sep 30 '19 at 15:04
  • Re, "They say that double-check...prevents two concurrent threads...make two different objects." It's the _locking_ that prevents threads from creating multiple instances. The _double-check_ part is supposed to be an optimization. It used to work on any computer that had only a single CPU, but it has a fatal flaw if you try to use it on a multi-processor machine (i.e., on most modern workstations, servers, and mobile phones.) – Solomon Slow Sep 30 '19 at 15:13
  • I am also thinking about the same. But It will create different instance for different process. – saurabh landge Sep 30 '19 at 15:19
  • No good reason for a Singleton pattern; even less for double checked locking. https://www.java67.com/2016/04/why-double-checked-locking-was-broken-before-java5.html – duffymo Sep 30 '19 at 15:52

4 Answers4

0

You don't want 2 threads to execute the code at the same time. It's a singleton. Singleton. There's only ever supposed to be one instance of the object at any given time. If two threads ran the same code at once, they could accidentally create two searchBoxes, and then it wouldn't be a singleton, would it?

What the synchronized there allows you to do is guarantee that the initialization of the singleton is thread-safe (not serialization-safe or all those other nice things you get with an enum-based singleton). By thread-safe, it means that you won't get 2 threads running the code at the same time. They are perfectly well allowed to run the code one after another, at which point the second thread will not re-instantiate the singleton, as it will have already been visibly (to the second thread) initialized.

Avi
  • 2,611
  • 1
  • 14
  • 26
  • I agree with you point. But as per threading concept, at a time an only single thread gets executed by the thread scheduler. Then how 2 threads will try to execute the above code. – saurabh landge Sep 30 '19 at 15:04
  • 1
    @saurabhlandge Well, that's where you're getting things mixed up. 2 threads can, will, and do run at the same time. Otherwise, you'd never even need the `synchronized` keyword at all. – Avi Sep 30 '19 at 15:14
0

the classloader will load static fields first, so the instance will be available before any thread will call the method getInstance() so for an effective and clean Singleton consider using this technique and avoid all expensive synchronized

public class SearchBox{
private static SearchBox instance = new SearchBox();
private SearchBox(){throw new OperationNotAllowedException();}
public static SearchBox getInstance(){ 
return instance;
}
public SomeReturn instanceMethod(){return ...} 
heyhooo
  • 82
  • 6
0

The code snippet you posted is thread-safe. Even if you remove outer if condition, there will be no problem.

if (searchBox == null) { //1
  synchronized (SearchBox.class) {
    if (searchBox == null) {  //2
        searchBox = new SearchBox();
        }
   }
}

Then why do we need outer if condition?
To improve performance.

Let's say we do not have first if block and searchbox object is already created. Threads will get blocked due to synchronized block. In such scenarios outer if block will stop thread entering the waiting stage.

Alternate approach, you can use a static inner class to implement Singleton pattern

public class Singleton  {    
    private static class SingletonHolder {    
        public static final Singleton instance = new Singleton();
    }    

    public static Singleton getInstance() {    
        return SingletonHolder.instance;    
    }    
}

Source for the above snippet: why is static inner class singleton thread safe

Govinda Sakhare
  • 5,009
  • 6
  • 33
  • 74
  • Correct. But what is the use of inner if condition? As we are checking instance is null or not. In outer loop we are checking instance is null or not. If it is null then the thread will acquire the lock on class and create the instance. As per thread scheduler, only one thread gets executed at a time. – saurabh landge Sep 30 '19 at 15:07
  • @saurabhlandge what if two threads concurrently enter the method and check the outer if condition at the same? in such case inner if condition act as a safe point. – Govinda Sakhare Sep 30 '19 at 16:08
0

Double-checked locking is fatally flawed. The problem is, this statement almost certainly assigns multiple variables:

searchBox = new SearchBox();

Besides the assignment to searchBox, there will also be several other assignments to initialize the new SearchBox instance.

Any other thread that attempts to look at searchBox without synchronization, is liable to see those assignments happen in a different order from how they happened in the thread that created the singleton.

That means, if thread A creates the singleton, and then thread B comes along and subsequently finds searchBox != null, then thread B will not enter the synchronized block, and thread B could see the singleton object in an uninitialized or a partially initialized state.

A singleton object must be safely published. Andy Turner's comment on the original question (see above) names several different ways to do that.


P.S.: You can "fix" double-checked locking by declaring the searchBox variable to be volatile, but then the cost of accessing searchBox becomes almost as high as the cost of locking the lock. Better just to use one of the safe publication patterns mentioned above.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57