CODEWITHSUNDEEP
javathread-safetyrace-condition
znlyj
znlyj

Reputation: 1149

I want to test if lazy initialization is thread-safe

I want to test if lazy initialization is thread-safe, so my code is below:

package LazyInit;

import java.util.Random;

public class UnThreadSafeLazyInit {

    private ExpensiveObject instance = null;

    public ExpensiveObject getInstance() {
        if (null == instance) {
            instance = new ExpensiveObject();
        }
        System.out.println("instance=" + instance);
        return instance;
    }

    /**
     * @param args
     */
    public static void main(String[] args) {
        // TODO Auto-generated method stub


        for (int i = 0; i < 5; i++) {
            UnThreadSafeLazyInit  init = new UnThreadSafeLazyInit();
            Task t1 = init.new Task();
            Task t2 = init.new Task();
            t1.start();
            t2.start();
            try {
                Thread.sleep(4000);
            } catch (Exception e) {
                e.printStackTrace();
            }
            System.out.println(t1.getInstance() == t2.getInstance());
        }
    }

    static class ExpensiveObject {

    }

    class Task extends Thread {

        private ExpensiveObject instance = null;
        private Random rand = new Random(47);

        public void setInstance () {
            this.instance = UnThreadSafeLazyInit.this.getInstance();
        }

        public ExpensiveObject getInstance() {
            return instance;
        }

        @Override
        public void run() {
            // TODO Auto-generated method stub

            try {
                Thread.sleep(rand.nextInt(1000));
            } catch (Exception e) {
                e.printStackTrace();
            }
            setInstance();
        }
    }

}

In my code, every time I new two Thead task to call public ExpensiveObject getInstance(), in order to prove the two instance maybe not the same reference to ExpensiveObject since race Condition. When I ran it, it always return true by t1.getInstance() == t1.getInstance(). As I know, if I don't synchronized the function public ExpensiveObject getInstance(), it could be return false since race Condition exists in Lazy Initialization. I need to find out which code is error. Thank you.

Upvotes: 0

Views: 207

Answers (4)

assylias
assylias

Reputation: 328608

Why it is not thread safe has already been covered by others. But I wanted to comment on your title: "I want to test if lazy initialization is thread-safe".

You can't test that a piece of code is thread safe. You might be able to find a test that proves that it is not, but testing only can't prove thread safety:

  • your test might not interleave threads in a way that reproduces the problem
  • your test might introduce additional synchronization (for example System.out.println is synchronized) that hides the actual issues
  • the issue might only appear in a very rare scenario that a few test runs will probably not encounter
  • the issue might only appear on certain JVMs / CPUs and the fact that your tests "works" with one specific setup does anyway not prove anything

Upvotes: 1

johnchen902
johnchen902

Reputation: 9599

It's not Thread-safe. You're just lucky this time. Modify your code:

public ExpensiveObject getInstance() {
    if (null == instance) {
        System.out.println("old instance=" + instance);
        instance = new ExpensiveObject();
        System.out.println("new instance=" + instance);
    }
    return instance;
}
// In main
Thread.sleep(40); // Thread.sleep(4000);
// In run
Thread.sleep(rand.nextInt(10)); // Thread.sleep(rand.nextInt(1000));

I see a lot of false in my console with this code.

Upvotes: 0

Peter Lawrey
Peter Lawrey

Reputation: 533520

It is not thread safe, just by inspection of the code. The problem you have is that delays of many milli-seconds is an enormous time to a computer and you are very, very unlikely to see a problem with this type of testing.

For example, the typical delay between updating a volatile field and it being visible to other threads is around 5 nano-seconds. It is for about this long that your solution is not thread safe. You are waiting up up to 1,000,000,000 nano-seconds to see if you get an issue.

This is like trying to see if fireworks lasting 5 seconds went off, but closing your eyes 317 years before concluding there was no fireworks.

Upvotes: 2

JB Nizet
JB Nizet

Reputation: 691745

The easiest way would be to make ExpensiveObject a really expensive object:

public class ExpensiveObject {
    public ExpensiveObject() {
        System.out.println("I'm expensive!");
        try {
            Thread.sleep(2000L);
        }
        catch (InterruptedException e) {
        }
        System.out.println("See. It took 2 seconds to create me!");
    }
}

Otherwise, the chance of entering into a rece condition is very small, especially since one thread is started after the other one, and thus calls setInstance() after the other one.

Upvotes: 0

Related Questions