10

This is a sample of the basic pattern I've been using for a Factory that returns a thread-safe Singleton:

public class UserServiceFactory {

    private volatile static UserService userService;

    private UserServiceFactory() { }

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

        return userService;
    }

}

It uses both volatile and the double check idiom to ensure that a single instance is created and is visible across threads.

Is there a less verbose and/or less expensive way to accomplish the same goal in 1.6+.

Dave Maple
  • 8,102
  • 4
  • 45
  • 64
  • possible duplicate of [Efficient way to implement singleton pattern in Java](http://stackoverflow.com/questions/70689/efficient-way-to-implement-singleton-pattern-in-java) – skaffman May 22 '11 at 13:39
  • 2
    with respect, i'd ask that the question not be closed as a duplicate of the aforementioned. I've provided a specific code example and asked for possible enhancements based on recent and near-future changes to the Java memory model. If this type of questioning is not allowed based on a more general previous question then we are losing some value to end-users -- no? – Dave Maple May 22 '11 at 13:46
  • 1
    See also http://stackoverflow.com/questions/157198/double-checked-locking-article and http://stackoverflow.com/questions/1625118/java-double-checked-locking – matt b May 22 '11 at 14:07

3 Answers3

19

Use the Initialization On Demand Holder idiom, it's simpler and better readable:

public class UserServiceFactory {

    private UserServiceFactory () {}

    private static class UserServiceHolder {
        private static final UserService INSTANCE = new UserService();
    }

    public static UserService getInstance() {
        return UserServiceHolder.INSTANCE;
    }

}

However, I'd prefer Just Create One idiom.


Update: as your question history confirms, you're using Java EE. If your container supports it, you could also make it a @Singleton EJB and use @EJB to inject it (although @Stateless is preferable since @Singleton is by default read-locked).

@Singleton
public class UserService {}

with e.g. in a JSF managed bean

@EJB
private UserService userService;

This way you delegate the instantiation job to the container.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
  • No it's not since Java 1.5. Read up on the changes to volatile that make it work. – Voo May 22 '11 at 13:36
  • @Voo: I realized that and I removed the phrase. – BalusC May 22 '11 at 13:38
  • Yeah, the volatile change was long due. Made multithreaded programming at least a bit simpler. Anyways +1 for the intelligent use of the inner class - letting the VM do the complex work behind the scenes is always a good idea ;) – Voo May 22 '11 at 13:41
  • This example looks very much like the optimization I was imagining. INSTANCE is declared static final so if UserService takes a long time to construct by thread1 then thread2 waits I assume? – Dave Maple May 22 '11 at 14:00
  • @Dave: Yes, the classloader guarantees threadsafety on `static` fields. It's invoked only once. – BalusC May 22 '11 at 14:02
  • 1
    @BalusC -- Love it, that's just what I was looking for. Thank you very much. Also enjoyed the reading on the "JustCreateOne" idiom. Definitely some food for thought in there. – Dave Maple May 22 '11 at 14:05
3

You could let the class loader do its maigc and initialize the static variable at startup - this is guaranteed to work, because the classloader guarantees single threaded behavior.

If you want to initialize the instance lazily and mostly lockfree, then no, you have to do it this way and make sure you're using Java >= 1.5

Edit: See BalusC's solution that uses the classloader a bit more intelligently. Note that this all works because the classloader initializes classes lazily - ie they're only loaded at their first access - and because inner classes are handled just like normal classes in that regard (just because you load the outer class doesn't mean the inner class is loaded)

Voo
  • 29,040
  • 11
  • 82
  • 156
0

Why not just

public synchronized static UserService getInstance() {
    if (userService == null) {
        userService = new UserServiceImpl();    
    }
    return userService;
}
Ferguzz
  • 5,777
  • 7
  • 34
  • 41
  • 2
    Because you always get the lock overhead although you only need it at the first invocation of the function. – Voo May 22 '11 at 13:30