2

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.

BeeOnRope
  • 60,350
  • 16
  • 207
  • 386

4 Answers4

1

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.

Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305
  • @irreputable: What is not lazy? Calling a constructor? – StriplingWarrior Mar 02 '11 at 21:22
  • @irreputable It's probably okay not being lazy. You can hide it behind a proxy if really necessary (unlikely). – Tom Hawtin - tackline Mar 02 '11 at 21:27
  • Assume also that the calling code cannot be changed. I want to maintain the contract that whatever consuming code asking for Pi handles the exception should one occur when creating the Pi. Unfortunately, I don't have the luxury of rearchitecting the many calls of this code. Also, what about callers from, for example, static methods? I guess those are evil and I need to remove them too just to implement this straightforward singleton... – BeeOnRope Mar 02 '11 at 21:40
  • @BeeOnRope Code that is using singletons, or other forms of mutable static is broken. No need to go any further than that. – Tom Hawtin - tackline Mar 02 '11 at 21:51
  • Consider the mutable static an implementation detail then. Maybe the bar() method is not static, but chooses to use the private static as a hidden implementation detail - the class could be replaced with another class with the same interface, which uses whatever DI magic is appropriate for testing or whatever. In this case the static isn't even really "mutable" in a practical sense - it can only assume a single-non null value. I just want to propagate errors to the called. Just like the hashcode in String is conceptually immutable (as are Strings), but is mutable for performance reasons. – BeeOnRope Mar 02 '11 at 21:57
  • @BeeOnRope Mutable statics are never implementation details. They are gaping flaws in the architecture. – Tom Hawtin - tackline Mar 02 '11 at 21:59
  • Can you elaborate a bit more than that? I assume you accept non-mutable statics, such as constants then? What about a constant which is very expensive to calculate, and may only be used in a handful of invocations of an application - can it not be calculated once, on demand, and stored in a mutable static? – BeeOnRope Mar 02 '11 at 22:04
  • @BeeOnRope I can't think of where I've seen an expensive to calculate constant. In any case, classes in Java are loaded lazily, so a `static final` constant is fine. – Tom Hawtin - tackline Mar 03 '11 at 13:37
  • Sure, but you may want to use a class without using a constant. The example which motivated this question is precisely an example where there is a constant object which is expensive to generate (or even if not expensive, which might throw a checked exception). – BeeOnRope Mar 09 '11 at 01:49
1

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.

StriplingWarrior
  • 151,543
  • 27
  • 246
  • 315
  • This is not lazy. The `Pi` is initialized before it's being used. – irreputable Mar 02 '11 at 21:14
  • 1
    Double checked locking was unsafe before the release of Java 5.0 - more than six years ago! Nowadays, it's sufficient to declare the field `volatile`. (c.f. http://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java ) – meriton Mar 02 '11 at 21:25
  • op seems to be very performance conscious. `spring.getInstance(name)` is a million time slower. – irreputable Mar 02 '11 at 21:28
  • @irreputable: without knowing more about what he's using it for, there's not much more I can say. Most of the time it doesn't pay to expect lazy instantiation, and most of the time it doesn't pay to be extremely performance-conscious. The patterns I've given are my recommendation based on my personal experience. I'm curious to hear your recommendation as well. – StriplingWarrior Mar 02 '11 at 21:37
  • In this case I'm not really interested in the *lazy* part for performance reasons - but rather to propagate the error to the first caller (rather that innocent other callers who might access Foo for other reasons). So in fact solution which are not lazy but preserve that behavior are interesting too. The underlying reason to use a singleton is that getPi() is an expensive call. – BeeOnRope Mar 02 '11 at 21:47
  • Further to the DI topic, the "getPi()" call is in a third-party framework which cannot necessarily be modified to accommodate the Provider.get(Pi.class) idiom (depending on what the underying IOC framework needs from its injected classes). Of course, I can create a wrapper object which accommodates IOC, but is that *really* the simple answer? – BeeOnRope Mar 02 '11 at 21:51
1

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;
}
irreputable
  • 44,725
  • 9
  • 65
  • 93
  • 1
    "JVM can do DCL safely, unfortunately, that power isn't available to Java programmers". Really? Then why does the wikipedia article on double checked locking write: "As of J2SE 5.0, this problem has been fixed. The volatile keyword now ensures that multiple threads handle the singleton instance correctly". – meriton Mar 02 '11 at 21:21
  • We can do DCL just fine with volatile since at least JDK 1.5. – BeeOnRope Mar 02 '11 at 21:23
  • I'm not sure what you mean - what does mean that machine code "can do DCL without volatile"? volatile is a java language level concept and the JVM will map that into appropriate hardware instructions, memory barriers and optimization restrictions. In native code, java volatile doesn't exist, but the same concerns exist. So the JVM can't do anything magic at class initialization time to avoid the costs of correct ordering. In modern implementations on x86 and x86-64, volatile reads don't require any special operations - they are regular reads, so they are very cheap. – BeeOnRope Mar 09 '11 at 01:44
  • I am accepting this answer, since it is the only that tried to answer the specific problem posed, rather than "don't use XYZ" or "use a DI framework". – BeeOnRope Mar 09 '11 at 01:48
  • @BeeOnRope What I mean is this. JVM's own DCL is a little cheaper than Java's `volatile` whose semantics is a little bit heavier than necessary for DCL. Doug Lee is working on bringing more memory primitives to Java (as concurrent library), so in future we could do DCL without `volatile` (or `final`). Saving a `volatile` read may or may not be significant, but it's still a desirable goal. – irreputable Mar 09 '11 at 02:13
  • looks like people developed a dependency on "dependency injection" framework, and they no longer know or care to know how to program basic stuff. it's a conspiracy:) – irreputable Mar 09 '11 at 02:18
  • Actually I think volatile is the minimum required to do DCL - release semantics on the read, and acquire on the read. Can you point what weaker semantics would allow DCL to remain safe? Technically the release on the write isn't needed, I suppose, since the synchronized block implies it, but we're really interested in the read, since the write happens only once. So I still dispute that the internal stuff can fundamentally be cheaper than DCL. – BeeOnRope Mar 09 '11 at 02:35
  • 1
    @BeeOnRope minimally, once a thread reads a non-null, it doesn't need to read it again. VM may optimize off a non-volatile read. it's very difficult to optimize off a volatile read; that requires global analysis. – irreputable Mar 09 '11 at 05:58
1

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.

meriton
  • 68,356
  • 14
  • 108
  • 175
  • 1
    before field access, there is a class access, which can fail, if the class failed to initialize. – irreputable Mar 02 '11 at 21:22
  • Are you sure about that? Did you try it? A static field access can most certainly throw an exception. – BeeOnRope Mar 02 '11 at 21:36
  • More about why it is needed - the Pi object is a stately singleton object which implements something like a parser. This parser is expensive to create, so I wish to create only one of them. The consumers cannot be changed, so the signature and functionality of bar() should remain the same. Even if the consumers could be changed, I'd argue that this is a simple and appropriate pattern (compared to say DI). – BeeOnRope Mar 02 '11 at 21:37