8

I just hit very strange (to me) behaviour of java. I have following classes:

public abstract class Unit {
    public static final Unit KM = KMUnit.INSTANCE;
    public static final Unit METERS = MeterUnit.INSTANCE;
    protected Unit() {
    }
    public abstract double getValueInUnit(double value, Unit unit);
    protected abstract double getValueInMeters(double value);
}

And:

public class KMUnit extends Unit {
    public static final Unit INSTANCE = new KMUnit();

    private KMUnit() {
    }
//here are abstract methods overriden
}

public class MeterUnit extends Unit {
    public static final Unit INSTANCE = new MeterUnit();

    private MeterUnit() {
    }

///abstract methods overriden
}

And my test case:

public class TestMetricUnits extends TestCase {

    @Test
    public void testConversion() {
        System.out.println("Unit.METERS: " + Unit.METERS);
    System.out.println("Unit.KM: " + Unit.KM);
    double meters = Unit.KM.getValueInUnit(102.11, Unit.METERS);
    assertEquals(0.10211, meters, 0.00001);
    }
}
  1. MKUnit and MeterUnit are both singletons initialized statically, so during class loading. Constructors are private, so they can't be initialized anywhere else.
  2. Unit class contains static final references to MKUnit.INSTANCE and MeterUnit.INSTANCE

I would expect that:

  • KMUnit class is loaded and instance is created.
  • MeterUnit class is loaded and instance is created.
  • Unit class is loaded and both KM and METERS variable are initialized, they are final so they cant be changed.

But when I run my test case in console with maven my result is:

 T E S T S

Running de.audi.echargingstations.tests.TestMetricUnits<br/>
Unit.METERS: m<br/>
Unit.KM: null<br/>
Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.089 sec <<< FAILURE! - in de.audi.echargingstations.tests.TestMetricUnits<br/>
testConversion(de.audi.echargingstations.tests.TestMetricUnits)  Time elapsed: 0.011 sec  <<< ERROR!<br/>
java.lang.NullPointerException: null<br/>
        at <br/>de.audi.echargingstations.tests.TestMetricUnits.testConversion(TestMetricUnits.java:29)
<br/>

Results :

Tests in error:
  TestMetricUnits.testConversion:29 NullPointer

And the funny part is that, when I run this test from eclipse via JUnit runner everything is fine, I have no NullPointerException and in console I have:

Unit.METERS: m
Unit.KM: km

So the question is: what can be the reason that KM variable in Unit is null (and in the same time METERS is not null)

HQCasanova
  • 1,158
  • 1
  • 10
  • 15
tomekK
  • 748
  • 8
  • 19
  • 2
    Its an anti-pattern.. you are using a child class as a field within the parent class – sanbhat Nov 08 '13 at 09:53
  • Ok, I can understand that it's antipattern, but here I am not asking about design, but what can be the reason that 1. Unit.KM is null and 2. it is null in with maven but not in eclipse – tomekK Nov 08 '13 at 09:54
  • Assertion don't fails. I know about precision, and that's why my assertion contains delta as third parameter. – tomekK Nov 08 '13 at 09:55
  • is Unit null or is KM null – Enigma Nov 08 '13 at 09:55
  • Unit is not null. Unit.KM is null – tomekK Nov 08 '13 at 09:56
  • 1
    If your JUnit runner does not fail, maybe it your test configuration launcher in for instance maven or gradle or *any build process* that is the actual problem. – zenbeni Nov 08 '13 at 09:58
  • is it the complete stacktrace? – siledh Nov 08 '13 at 09:58
  • @siledh yes, its whole stacktrace. NullPointerException doesn't comes from inside the function invocation but from test case – tomekK Nov 08 '13 at 10:00
  • You are using Unit as a Facade to get your singletons, but it is already an abstract class... not a good separation of concerns, @sanbhat said it is anti-pattern (with some kind of cyclic dependency), you should really consider changing that and add a true UnitFacade class for instance that holds these singletons (or use an IOC framework). – zenbeni Nov 08 '13 at 10:03
  • I copied you code and run mvn test,there is no npe ... – farmer1992 Nov 08 '13 at 10:03
  • @zenbeni ok thanks, I will probably do that. Still I am really curious what can be the reason of such behaviour. – tomekK Nov 08 '13 at 10:04
  • 1
    Are you using a forkMode in surefire if you are using Maven? Maybe you should try with forkMode=none to see if concurrency in test execution is the problem. – zenbeni Nov 08 '13 at 10:09
  • hmm... now I am confused. In the meantime I removed those static variables from Unit class and changed to use INSTANCE from MeterUnit and KMUnit (no NPE of course) and installed this module in local remo. And after @zenbeni comment I wanted to get back to original code to check if forkMode would help. But I have no NPE error now... so it has to be something with... I don't know - maven repo? – tomekK Nov 08 '13 at 10:22
  • "but here I am not asking about design" 90% of programming is _about_ design! A good design would prevent problems like yours from happening in the first place. – AJMansfield Nov 11 '13 at 23:12
  • Why not just use an enum? – Mechanical snail Dec 07 '13 at 03:12

2 Answers2

5

Static initialization can be tricky. You have an interdependency between A -> B and B -> A. The reason this is a bad idea is because of how the JVM starts loading statics from the top down in a class - if it hits a new class it has yet to initialize, it waits until it initializes that class, and its dependencies, recursively, until everything is ready, then proceeds.

Except when it's already loading a class. If A refers to B, and B refers to A, it can't start loading A a second time, or it would be an infinite loop (because A would load B again, which loads A). So, in that case, it basically says "already started loading that, nothing more to do, continuing on".

Moral of the story: depending on the ordering of the loading of classes, KMUnit.INSTANCE may not be initialized when you hit this line:

public static final Unit KM = KMUnit.INSTANCE;

Imagine that you're the JVM and you start loading KMUnit. It would have to load Unit, the first time it sees it, in order to, e.g. create a object that is a subclass of Unit when we get to the first creation (or perhaps before - I'm a touch foggy on JVM static loading). But that in turn triggers initialization of statics in Unit including this:

public static final Unit KM = KMUnit.INSTANCE;
public static final Unit METERS = MeterUnit.INSTANCE;

Ok. Now Unit is done loading, and we finish constructing a KMUnit for KMUnit.INSTANCE... but wait - we already set KM = KMUnit.INSTANCE, which was null at the time. So it remains null. Oops.

On the other hand, if Unit loads first, then it waits for KMUnit to load before initializing, so KMUnit.INSTANCE is set when we actually run the initializer.

I think. I'm a bit sleep deprived, and I'm not an expert in classloading.

James
  • 8,512
  • 1
  • 26
  • 28
0

I would expect that:

- KMUnit class is loaded and instance is created.
- MeterUnit class is loaded and instance is created.
- Unit class is loaded and both KM and METERS variable are initialized, they are final so they cant be changed.

Even without going into the the language spec it is easy to see why the above sequence is impossible.

KMUnit extends Unit. To create the static field KMUnit.INSTANCE its class KMUnit has to be created. And to create KMUnit its super class Unit has to be created. Finally to create the class Unit its static fields KM and METERS has to be assigned.

But we got here by trying to create the class KMUnit and we haven't loaded the class Meters yet. So it impossible that the static fields of the super class are assigned correct values (i.e. references to fully constructed object).

There are two problems in the steps you describe:

The error in the steps you describe is that you are delaying loading of Unit, which cannot be done.

Hope this helps. Static initializers are not easy to understand and their specs even less so. It is perhaps easier to rationalize why something cannot be done, hence my non-formal answer.

Miserable Variable
  • 28,432
  • 15
  • 72
  • 133