Which implementation for lazy singleton whose initialialization might fail?

446 views Asked by At

Imagine you have a static no-argument method which is idempotent and always returns the same value, and may throw a checked exception, like so:

class Foo {
 public static Pi bar() throws Baz { getPi(); } // gets Pi, may throw 
}

Now this is a good candidate for a lazy singleton, if the stuff that constructs the returned Object is expensive and never changes. One choice would be the Holder pattern:

class Foo {
  static class PiHolder {
   static final Pi PI_SINGLETON = getPi();
  }
  public static Pi bar() { return PiHolder.PI_SINGLETON; }
}

Unfortunately, this won't work because we can't throw a checked exception from an (implicit) static initializer block, so we could instead try something like this (assuming we want to preserve the behavior that the caller gets the checked exception when they call bar()):

class Foo {
  static class PiHolder {
   static final Pi PI_SINGLETON;
   static { 
    try { 
     PI_SINGLETON =  = getPi(); }
    } catch (Baz b) {
     throw new ExceptionInInitializerError(b);
    }
  }

  public static Pi bar() throws Bar {
   try {
    return PiHolder.PI_SINGLETON;
   } catch (ExceptionInInitializerError e) {
    if (e.getCause() instanceof Bar)
     throw (Bar)e.getCause();
    throw e;
   }
}

At this point, maybe double-checked locking is just cleaner?

class Foo {
 static volatile Pi PI_INSTANCE;
 public static Pi bar() throws Bar {
  Pi p = PI_INSTANCE;
  if (p == null) {
   synchronized (this) {
    if ((p = PI_INSTANCE) == null)
     return PI_INSTANCE = getPi();
   }
  }
  return p;
 }
}

Is DCL still an anti-pattern? Are there other solutions I'm missing here (minor variants like racy single check are also possible, but don't fundamentally change the solution)? Is there a good reason to choose one over the other?

I didn't try the examples above so it's entirely possible that they don't compile.

Edit: I don't have the luxury of re-implementing or re-architecting the consumers of this singleton (i.e., the callers of Foo.bar()), nor do I have the opportunity to introduce a DI framework to solve this problem. I'm mostly interested in answers which solve the issue (provision of a singleton with checked exceptions propagated to the caller) within the given constraints.

Update: I decided to go with DCL after all, since it provided the cleanest way to preserve the existing contract, and no one provided a concrete reason why DCL done properly should be avoided. I didn't use the method in the accepted answer since it seemed just to be a overly complex way of achieving the same thing.

4

There are 4 answers

8
irreputable On BEST ANSWER

The "Holder" trick is essentially double checked locking performed by JVM. Per spec, class initialization is under (double checked) locking. JVM can do DCL safely (and fast), unfortunately, that power isn't available to Java programmers. The closest we can do, is through a intermediary final reference. See wikipedia on DCL.

Your requirement of preserving exception isn't hard:

class Foo {
  static class PiHolder {
    static final Pi PI_SINGLETON;
    static Bar exception;
    static { 
      try { 
        PI_SINGLETON =  = getPi(); }
      } catch (Bar b) {
        exception = b;
      }
    }
  }
public Pi bar() throws Bar {
  if(PiHolder.exception!=null)
    throw PiHolder.exception;  
  else
    return PiHolder.PI_SINGLETON;
}
9
Tom Hawtin - tackline On

i strongly suggest throwing out the Singleton and mutable statics in general. "Use constructors correctly." Construct the object and the pass it into objects that need it.

6
StriplingWarrior On

In my experience, it's best to go with dependency injection when the object you're trying to get requires anything more than a simple constructor call.

public class Foo {
  private Pi pi;
  public Foo(Pi pi) {
    this.pi = pi;
  }
  public Pi bar() { return pi; }
}

... or if Laziness is important:

public class Foo {
  private IocWrapper iocWrapper;
  public Foo(IocWrapper iocWrapper) {
    this.iocWrapper = iocWrapper;
  }
  public Pi bar() { return iocWrapper.get(Pi.class); }
}

(the specifics will depend somewhat on your DI framework)

You can tell the DI framework to bind the object as a singleton. This gives you a lot more flexibility in the long run, and makes your class more unit-testable.

Also, my understanding is that double-checked locking in Java is not thread safe, due to the possibility of instruction reordering by the JIT compiler. edit: As meriton points out, double-checked locking can work in Java, but you must use the volatile keyword.

One last point: if you're using good patterns, there's usually little or no reason to want your classes to be lazily instantiated. It's best to have your constructor be extremely light-weight, and have the bulk of your logic be performed as part of the methods. I'm not saying you're necessarily doing something wrong in this particular case, but you may want to take a broader look at how you're using this singleton and see if there's not a better way to structure things.

3
meriton On

Since you don't tell us what you need this for it's kind of hard to suggest better ways of achieving it. I can tell you that lazy singletons are rarely the best approach.

I can see several issues with your code, though:

try {
    return PiHolder.PI_SINGLETON;
} catch (ExceptionInInitializerError e) {

Just how do you expect a field access to throw an exception?


Edit: As Irreputable points out, if the access causes class initialization, and initialization fails because the static initializer throws an exception, you actually get the ExceptionInInitializerError here. However, the VM will not try to initialized the class again after the first failure, and communicate this with a different exception, as the following code demonstrates:

static class H {
    final static String s; 
    static {
        Object o = null;
        s = o.toString();
    }
}

public static void main(String[] args) throws Exception {
    try {
        System.out.println(H.s);
    } catch (ExceptionInInitializerError e) {
    }
    System.out.println(H.s);
}

Results in:

Exception in thread "main" java.lang.NoClassDefFoundError: Could not initialize class tools.Test$H 
        at tools.Test.main(Test.java:21)

and not an ExceptionInInitializerError.


Your double checked locking suffers from a similar problem; if construction fails, the field remains null, and a new attempt to construct the PI is made every time PI is accessed. If failure is permanent and expensive, you might wish to do things differently.