2

My understanding of volatile is that it ensures that the value is always read from memory, so as far as I can see, in the following example, the myObject variable would need to be volatile to avoid a NullPointerException being raised:

private final Object lock = new Object();
private MyObject myObject = null;

//...

synchronized (lock) {
    if (myObject == null) {
        myObject = new MyObject();
    }

    myObject.blah();

    // some other stuff that I want synchronized
}

myObject is only ever touched in the synchronized block. lock is only every used to synchronize that block.

Is that correct?

So rephrased slightly, my question is...imagine two threads are hitting that code. First thread locks and sets myObject, calls .blah() and any other code within the synchronized block and exits the synchronized block. This allows thread two to enter the synchronized block. Without setting myObject to volatile, is there are chance it could still evaluate myObject == null to true?

Cheetah
  • 13,785
  • 31
  • 106
  • 190
  • Is `myObject` volatile? Also, is `Object lock` *constant* for all instances of your class? – Luiggi Mendoza Jun 22 '14 at 21:09
  • Is there any other code that can set `myObject` to `null`? – Sotirios Delimanolis Jun 22 '14 at 21:20
  • @SotiriosDelimanolis - no imagine this code is self contained. – Cheetah Jun 22 '14 at 21:21
  • I think there might be a chance, I've had not so happy experiences using `synchronized` blocks, where unexpected results happened and synchronization didn't work, especially at the beginning of a process. Better use `ReentrantLock`. And as for `volatile`, it has nothing to do with synchronization in this case, so not needed. – dabadaba Jun 22 '14 at 21:25
  • @dabadaba - I am not really asking about synchronization...more whether I can ensure that I don't get a NPE accessing `myObject`. – Cheetah Jun 22 '14 at 21:30
  • I think you can get the exception, `synchronized` blocks are not perfect. – dabadaba Jun 22 '14 at 21:32
  • If `Object lock` has the same instance for all the threads this reference will be used, then there's no need to mark `myObject` as `volatile`. – Luiggi Mendoza Jun 22 '14 at 21:32
  • @dabadaba that would be a [cosmic ray](http://stackoverflow.com/q/2580933/1065197). – Luiggi Mendoza Jun 22 '14 at 21:50

2 Answers2

4

The synchronized block will ensure the updates to memory is seen by other threads. There is no need to make myObject volatile.

From Intrinsic Locks and Synchronization:

When a thread releases an intrinsic lock, a happens-before relationship is established between that action and any subsequent acquistion of the same lock.

nos
  • 223,662
  • 58
  • 417
  • 506
  • I've been doing some research, as I wondered why I thought you needed the volatile keyword in there. I believe it was to do with DCL, this: http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html. Now am I right in saying because I don't have the check for null on the outside of my synchronization loop, I don't need the check? – Cheetah Jun 23 '14 at 07:13
0

I think volatile is not needed here, because every thread which goes into synchronized block is checking myObject references, so myObject should be instantieted when first thread goes into block, other threads are secured by checking is myObject not null. For me looks good.

EDIT: I hope there is only this one block where you want to use myObect reference, and you do not change this reference before or after synchoronize block.

tomasz.wojciechowski
  • 54
  • 4
  • But is there not a chance that the value could sit in a cache somewhere and therefore result in an NPE? – Cheetah Jun 22 '14 at 21:31
  • No. There is no such posibility. – tomasz.wojciechowski Jun 22 '14 at 21:36
  • In code which you paste, every thread (but only one at the time) will check myObject value, then run blah method, so every other thread will wait. But you cannot implement second block where you set myObject=null, because then will happen something like you said. If you want to set null to myObject you need to do it in one block. – tomasz.wojciechowski Jun 22 '14 at 21:42