CODEWITHSUNDEEP
javaoopsingletondouble-checked-locking
Pavel Petrashov
Pavel Petrashov

Reputation: 1262

How can I create the correct lazy singleton?

I have this class

public class Singletons {
    private static UserService userService;
    private static OrderService orderService;

    public static UserService getUserService() {
        if (userService == null) {
            synchronized (Singletons.class) {
                if (userService == null)
                    userService = new UserService();
            }
        }

        return userService;
    }

    public static OrderService getOrderService() {
        if (orderService == null) {
            synchronized (Singletons.class) {
                if (orderService == null)
                    orderService = new OrderService();
            }
        }

        return orderService;
    }
}

Intellij IDEA shows me this warrnig:

Double-checked locking tries to initialize a field on demand and in a thread-safe manner while avoiding the cost of synchronization. Unfortunately it is not thread-safe when used on a field that is not declared volatile. When using Java 1.4 or earlier, double-checked locking doesn't work even with volatile fields. Read the article linked above for the detailed explanation of the problem.

And this link: The "Double-Checked Locking is Broken" Declaration

I read this article but I can not understand how I can fix my class. As I understood I can do the following:

public class Singletons {
    private static volatile UserService userService;
    private static volatile OrderService orderService;

    public static UserService getUserService() {
        if (userService == null) {
            synchronized (Singletons.class) {
                if (userService == null)
                    userService = new UserService();
            }
        }

        return userService;
    }

    public static OrderService getOrderService() {
        if (orderService == null) {
            synchronized (Singletons.class) {
                if (orderService == null)
                    orderService = new OrderService();
            }
        }

        return orderService;
    }
}

Is this enough?(EDDIT - I use java > 1.4 so it is not for me) Or the second approach might be like this:

public class Singletons {
    private static final UserService userService = new UserService();
    private static final OrderService orderService = new OrderService();

    public static UserService getUserService() {
        return userService;
    }

    public static OrderService getOrderService() {
        return orderService;
    }
}

But it looks strange. What if I need to pass other classes to the constructor? It looks wrong. Maybe this, like in this answer(StackOverflow answer):

public class Singletons {
    private static class Ref {
        static final UserService userService = new UserService();
        static final OrderService orderService = new OrderService();
    }

    public static UserService getUserService() {
        return Ref.userService;
    }

    public static OrderService getOrderService() {
        return Ref.orderService;
    }
}

But it looks the same as the previous one. So How can I create the correct lazy singleton?

Upvotes: 1

Views: 159

Answers (1)

Eugene
Eugene

Reputation: 5985

There are multiple ways to fix this. The simplest one is to simply not use double-checked locking at all and synchronize the whole method instead. With early versions of the JVM, synchronizing the whole method was generally advised against for performance reasons. But synchronized performance has improved a lot in newer JVMs, so this is now a preferred solution. If you prefer to avoid using synchronized altogether, you can use an inner static class to hold the reference instead. Inner static classes are guaranteed to load lazily.

Solution 1. Synchronized method

public class Singletons {
    private static UserService userService;

    public static synchronized UserService getUserService() {
        if (userService == null) {
            userService = new UserService();
        }
        return userService;
    }
}

Solution 2. Nested static class
The solution which you described, but split instances into several static inner classes. It will guarantee that each instance will be created on-demand. In your example, two instances will be created when you call one of them. Class initialization occurs the first time we use one of its methods or fields. Java Virtual Machine is responsible for taking care of synchronization and recursive initialization. See documentation.

public class Singletons {
    private static class UserServiceRef {
        static final UserService userService = new UserService();
    }

    private static class OrderServiceRef {
        static final OrderService orderService = new OrderService();
    }

    public static UserService getUserService() {
        return UserServiceRef.userService;
    }

    public static OrderService getOrderService() {
        return OrderServiceRef.orderService;
    }
}

As for me Solution 2 more preferable because it is not using synchronized. JWM is responsible for the synchronization and classes are guaranteed to load lazily.

Upvotes: 1

Related Questions