5

Do I understand right that end of constructor is not a happens - before relation in Java? Is it possible, that code below with threads A and B not been synchronized somehow could throw a NullPointerException ?

// Shared reference declaration
public MyClass val;

// Class declaration
public class MyClass {
    public Object object;
    public MyClass() {
        object = new Object();
    }
}

// Using in thread A
MyClass loc = new MyClass();
val = loc;

// Using in thread B
if(val != null) {
    val.object.hashCode(); // IMO could throw NPE
}
Nulldevice
  • 3,926
  • 3
  • 31
  • 37
  • I know that final will make NPE not possible. Let me explain why there is no final in my case. Imagine a data object that two threads use to exchange messages. There are a lot of fields in this data object, and many of them could have a default value in ready message. Creating of messages using constructors is not convenient, Builder pattern is much more convenient. That is why fields are not final in my case. – Nulldevice Jun 09 '14 at 08:43

2 Answers2

3

If val was marked final (which would also prevent a subsequent assignment to null) then an NPE would not be possible:

public final MyClass val = new MyClass();

Otherwise, yes, your code is brittle.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • Why would final work? Does final add a memory barrier? – Joachim Isaksson Jun 09 '14 at 08:36
  • Final prevents `val` from being assigned to something else; including `null`. – Bathsheba Jun 09 '14 at 08:36
  • 3
    As far as I can see, the problem isn't `val`, it's that the result of the MyClass constructor isn't guaranteed to be seen by another processor without a memory barrier. As far as the other processor is concerned, it may not have finished constructing before it's used. My view on the Java memory model may be a bit out of date though. – Joachim Isaksson Jun 09 '14 at 08:38
  • @Bathsheba: problem is reading val, before its referring to anything not null. NullDevice you need to insure that the val is assigned before using it. You need to guard those two statements in thread A. or wait in thread B till val is assigned. or check for null in Thread B in a loop, & break out when val is null. – Shailesh Aswal Jun 09 '14 at 08:49
  • Absolutely: so don't ever give anything the chance of reading `val` before it's assigned to something! – Bathsheba Jun 09 '14 at 08:51
  • correction: *break out when you find not null in your check. – Shailesh Aswal Jun 09 '14 at 08:56
  • To nitpick this is not correct. If `object` was final we'd be in the clear. Whether making `val` final is enough depends on the constructor of the class that uses it - if we publish the instance before the constructor is finished we *do* have a problem. Obviously bad practice, but it is possible. – Voo Jun 11 '14 at 23:06
3

If it was

 val.object.hashCode();

then there was a possibility of NPE, since while thread B may see val = loc it may not have seen object = new Object(); yet due to cache behaviour on different cores etc. which is allowed by Java's weak memory model for performance reasons.

I don't think your original code can throw NPE since if val is not null then hashCode will execute.

if(val != null) {
    val.hashCode(); // IMO could throw NPE
}
auselen
  • 27,577
  • 7
  • 73
  • 114