5

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?

Improve this question
14
  • Best way to avoid it: Don't use singletons. Commented Jun 3, 2014 at 18:12
  • Like whatsisname mentions, it's best not to use a singleton; they're essentially a global variable. Additionally, ask yourself if you really need to use inheritance; favor composition. Commented Jun 3, 2014 at 18:16
  • 2
    @Prog If you're accessing an immutable value, that's fine, although I question the need for lazy initialization. I guess the bigger question is, why do you have a cyclic dependency and do you really need to use inheritance? Commented Jun 3, 2014 at 18:24
  • 3
    This problem has nothing to do with a singleton. You would have the same problem even if Singleton was not a singleton. Creating a new S causes A's constructor to be called which causes B's constructor to be called which causes a new S to be created which causes A's constructor to be called which causes B's constructor to be called which causes a new S to be created which causes A's constructor...... Commented Jun 3, 2014 at 21:52
  • 3
    You avoid this problem by drawing a class diagram and being aware of dependencies. Cyclic dependencies should be eliminated. In your case, A knows about B. B knows about S which implicitly means it knows about A. That's a problem. There is seldom a need for 2-way dependencies. A class shouldn't know who contains it. Commented Jun 3, 2014 at 22:01

3 Answers 3

Reset to default
7

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.
Improve this answer
0
0

I can think of a couple of ways to break the cycle.

  1. Don't instantiate a CMajor when instantiating MusicalResources. Let a higher level function construct the necessary Scales. Any time a Scale is constructed, add a reference to MusicalResources.

  2. Don't query MusicalResources for the Notes when constructing an instance of CMajor. Query them on an as needed basis.

Improve this answer
0

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.)

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.