Let's imagine I have the following java class :

static class Singleton {static Singleton i;static Singleton getInstance() {if (i==null) {i=new Singleton();}return i;}}

Now, we all know this will work, but - it apparently is not thread safe - I am not actually trying to fix the thread safety - this is more of a demo, my other class is identical, but uses a mutex and synchronization - the unit test will be ran against each to show that one is thread safe, and the other, is not. What might the unit test which would fail if getInstance is not thread safe look like?

  • 1
    What aspect of it are you trying to test? If it's "exactly one instance will ever be created," that's not something a unit test can really do. You could write a test that starts a bunch of threads and has each one of them invoke getInstance(), and then check the instances you get back -- but even that is probabilistic at best, and likely won't hit the same conditions that you'll hit in real life (in terms of CPU usage, cache lines, etc). When it comes to checking for race conditions, unit tests are usually insufficient. That's one of the things that makes multithreading more difficult.– yshavitFeb 13 at 17:11
  • It might be hard to write a test that provokes an unsafe situation because you'd have to time 2 threads in a way that both enter the if-block simultaneously but one returns i before the other changes it. The problem is that you can't time threads in such a way, since when and how fast they run depends on a huge amount of things.– ThomasFeb 13 at 17:12
  • @yshavit - Yeah, thats why I cam to ask the question - I have had a bunch of thoughts on it, but I can't come up with anything deterministic!– MickeyThreeShedsFeb 13 at 17:13
  • I'd say writing a test that check for potentially unsafe situations requires a good understanding on what could go wrong so that you can try to provoke those situations. However, once you understand the situations you're normally able to "prove" why they're dangerous and that they can happen - in which case you'd not need the test anymore but should be able to fix the problem right away.– ThomasFeb 13 at 17:16
  • Create and start two threads which wait for a countdown latch before calling getInstance(); then count the latch down. You may have to run it many times (resetting i in between), but this is maybe the easiest way to exercise the race.– Andy TurnerFeb 13 at 17:25

Well, race conditions are by nature probabilistic so there's no deterministic way to truly generate a race condition. Any possible way against your current code would need to be run many times until the desired outcome is achieved. You can enforce a loose ordering of access on i by making a mock singleton to test against to simulate what a certain condition might look like, though. Rule of thumb with synchronization is preventative measures beat trying to test and figure out what's wrong after bad code is mangled in a code base.

static class Singleton {static Singleton i;static Singleton getInstance(int tid) {if (i==null) {if (tid % 2==0) i=new Singleton()}return i;}}

So certain threads will write to i and other threads will read i as if they reached "return i" before "the even thread id's were able to check and initialize i" (sort of, not exactly, but it simulates the behavior). Still, there's a race between the even threads in this case because the even threads may still write to i after another reads null. To improve, you'd need to implement thread safety to force the condition where one thread reads i, gets null, while the other thread sets i to new Singleton() a thread-unsafe condition. But at that point you're better off just solving the underlying issue (just make getInstance thread safe!)

TLDR: there are infinitely many race conditions that can occur in a unsafe function call. You can mock the code to generate a mock of a specific race condition (say, between just two threads) but it's not feasible to just blanket test for "race conditions"

    This code worked for me.The trick is that it is probabilistic like said by other users.So, the approach that should be taken is to run for a number of times.

    public class SingletonThreadSafety {public static final int CONCURRENT_THREADS=4;private void single() {// Allocate an array for the singletonsfinal Singleton[] singleton=new Singleton[CONCURRENT_THREADS];// Number of threads remainingfinal AtomicInteger count=new AtomicInteger(CONCURRENT_THREADS);// Create the threadsfor(int i=0;i<CONCURRENT_THREADS;i++) {final int l=i; // Capture this value to enter the inner thread class new Thread() {public void run() {singleton[l]=Singleton.getInstance();count.decrementAndGet();}}.start();}// Ensure all threads are done// The sleep(10) is to be somewhat performant, (if just loop,// this will be a lot slow. We could use some other threading// classes better, like CountdownLatch or something.)try { Thread.sleep(10); } catch(InterruptedException ex) { }while(count.get() >=1) {try { Thread.sleep(10); } catch(InterruptedException ex) { }}for( int i=0;i<CONCURRENT_THREADS - 1;i++) {assertTrue(singleton[i]==singleton[i + 1]);}}@Testpublic void test() {for(int i=0;i<1000;i++) {Singleton.i=null;single();System.out.println(i);}}}

    This have to make some change in the Singleton design pattern. That the instance variable is now accessible in the Test class. So that we could reset the Singleton instance available to null again every time the test is repeated, then we repeat the test 1000 times (if you have more time, you could make it more, sometimes finding an odd threading problem require that).

    • Unfortunately - modifying the class in order to make this work is not really an option - we are testing for faults in the current spec.– MickeyThreeShedsFeb 13 at 19:23
    • There are some solutions to expose the private field for Unit testing. This solution didn't require any other change in your class except that the instance field must be exposable and settable in the test environment. dzone.com/articles/unit-testing-private-methods . The simplest thing is to use "reflection" to set the instance field (i in your example) to zero after each iteration– user9335240Feb 13 at 19:26
    • Small note, but do you know about CountDownLatch? You're using AtomicInteger basically as a replacement for it.– yshavitFeb 13 at 21:09
    • @yshavit This is only for simplicity.– user9335240Feb 13 at 21:22
    • @user9335240 Up to you, but I don't think two try-sleeps and a loop are simpler than latch.await() :)– yshavitFeb 13 at 21:45

    Your Answer


    By posting your answer, you agree to the privacy policy and terms of service.

    Not the answer you're looking for? Browse other questions tagged or ask your own question.