I ran into an unusual error while working on my project. To better learn from and remember it, I’d like to know if this type of error has a name or some definition. (The error itself OutOfMemoryError isn’t unusual, I’m talking about what lead to this error).
My (simplified) situation was this:
I had a class Singleton:
class Singleton extends ClassA{
private static Singleton instance;
private Singleton(){ // implicitly calls super() }
public static Singleton getInstance(){
if (instance==null) instance = new Singleton();
return instance;
}
}
And it’s superclass ClassA:
abstract class ClassA{
public ClassA(){
ClassB objectB = new ClassB();
// .. some logic ..
}
}
And a class ClassB that uses Singleton in it’s constructor:
class ClassB{
public ClassB(){
Singleton singleton = Singleton.getInstance();
// .. some logic ..
}
}
What happens is the following:
1- ClassB is instantiated somewhere. It’s constructor is invoked and calls Singleton.getInstance().
2- Singleton.getInstance() sees that instance == null and executes instance = new Singleton().
3- The constructor of Singleton is run. It’s empty, but it implicitly calls the superclass’ (ClassA) constructor.
4- The superclass of Singleton instantiates a ClassB object new ClassB().
5- ClassB‘s constructor is invoked and calls Singleton.getInstance()…
Since the instantiation of instance in Singleton never reached it’s finish, instance == null still returns true, and the cycle never ends.
Resulting in an infinite loop, that finally resulted in an OutOfMemoryError.
So my question is: is this kind of infinite-loop-with-singletons error a common issue? Any ideas how I can avoid it in the future?
14
It is a simple circular dependency during initialization, independent of the Singleton. There are three classes that instantiate each other in a cycle: A instantiates B, which instantiates C, which in turn instantiates A again, and you have an endless instantiation cycle.
Another thing that needs to be evaluated is why do the objects have a circular dependency on each other regardless of instantiation. That is, even if somehow they were all instantiated, they should not still have a circular dependency since that is often indicative of high coupling and possibly other design issues.
Having said that, it seems that the Singleton is not yet completely implemented, because it does not guarantee singleton across multiple threads. When the locking necessary for a singleton implementation is coded, it will very quickly run into a deadlock.
Usually, these types of errors are found:
- At design time by looking at class interactions,
- At implementation time by working through the code in the head,
- At compile time as some compilers might detect and issue warnings/errors,
- At runtime, by getting the kind of exception this code raised.
0
I can think of a couple of ways to break the cycle.
-
Don’t instantiate a
CMajorwhen instantiatingMusicalResources. Let a higher level function construct the necessaryScales. Any time aScaleis constructed, add a reference toMusicalResources. -
Don’t query
MusicalResourcesfor theNoteswhen constructing an instance ofCMajor. Query them on an as needed basis.
One of the downsides of Singletons is how they interact with super and sub classes. Another downside of Singletons is nesting them — that gets messy fast. No offense, but I find it a little funny when you say, “I don’t see the problem…” when in fact you are seeing a problem that blows up your runtime engine 😉
Aside from that, your model is all kitty-wampus. Again, no offense, but…! I think, for example, you’re making an equivalence between a Scale and a Note that messes up the hierarchy. I’d take out the Singleton code first, and then step back and make a diagram of the is-a and has-a relationships (for example, it might be cleaner if Scale “has a” Note instead of “is a” Note)
It might be good to diagram it out, with the classes, subclasses, and instances laid out with arrows between them.
If you’re dishing out immutable reference objects, bundling them up as public static final members of some container class is a lot cleaner and less nastiness-prone than Singleton. Also, I’m not sure where, but I think that enums are going to be your friends here too.
(Enums are kind of the Java-approved version of immutable Singleton, again without the nastiness.)