I understand that you want to demonstrate that your code is not thread-safe and it may break.
That is true, the code is not thread-safe.
If there are two threads A and B that at the same time access getInstance
method, both may see that its null and both may get a different instance of the class.
However, the test that you are writing to demonstrate this is not perfect, to break thread-safety we need a race condition, and a race condition occurs when two or more threads try to access shared data or resource at exactly the same time.
In your SingletonDemo
you have used Executors
but you are iterating and spawning one thread at a time. Since a thread is spawned sequentially there are very fewer chances that two or more thread will collide in getting the instance at the same time.
What we need is that 2 threads collide at exactly the same time, this is possible if they are started at exactly the same time (at least this is the minimum condition we can create to increase our probability of collision).
For this we need something called as CyclicBarrier, I am taking references from a SO answer https://stackoverflow.com/a/3376628/2179336
Let's rewrite our demo now:
public class SingletonDemo {
public static void main(String[] args) throws InterruptedException, BrokenBarrierException {
final CyclicBarrier gate = new CyclicBarrier(3);
Thread t1 = new Thread() {
public void run() {
try {
gate.await();
Singleton.getInstance();
} catch (InterruptedException | BrokenBarrierException e) {
e.printStackTrace();
}
}
};
Thread t2 = new Thread() {
public void run() {
try {
gate.await();
Singleton.getInstance();
} catch (InterruptedException | BrokenBarrierException e) {
e.printStackTrace();
}
}
};
t1.start();
t2.start();
gate.await();
}
}
Here, we are trying to span 2 threads at the same time using Cyclic Barrier.
Now let's modify Singleton
class for better logging:
public class Singleton {
private static Singleton singleton = null;
private Singleton() {
}
public static Singleton getInstance() throws InterruptedException {
System.out.println("Requesting Thread" + Thread.currentThread()
.getName());
if (singleton == null) {
System.out.println("Created New Instance for " + Thread.currentThread()
.getName());
singleton = new Singleton();
}
return singleton;
}
}
There, now try running SingletonDemo multiple times and if you are lucky, you may experience that your class is not thread-safe and get logs like this:
Requesting ThreadThread-2
Requesting ThreadThread-1
Created New Instance for Thread-1
Created New Instance for Thread-2
This tells you that instance was created for both the threads hence proving that the thread-safety was breached.