Read field stale value after object construction
Asked Answered
C

4

19

I'm reading a book "Java concurrency in practice" by Brian Goetz. Paragraphs 3.5 and 3.5.1 contains statements that I can not understand.

Consider the following code:

public class Holder {
  private int value;
  public Holder(int value) { 
    this.value = value;
  }

  public void assertValue() {
    if (value != value) throw new AssertionError("Magic");
  }
}

class HolderContainer {
  // Unsafe publication
  public Holder holder;

  public void init() {
    holder = new Holder(42);  
  }
}

Author states that:

  1. In Java, Object constructor first writes default values to all fields before subclass constructor run.
  2. Therefore it's possible to see field default value as a stale value.
  3. Thread may see stale value the first time it reads a field and then a more up-to-date value the next time, which is why assertN  can throw AssertionError.

So, according to the text, with some unlucky timing it is possible that value = 0; and in the next moment value = 42.

I agree with point 1 that Object constructor firstly fills fields with default values. But I don't understand points 2 & 3.

Let's update authors code and consider the following example:

public class Holder {
  int value;

  public Holder(int value) {
    //Sleep to prevent constructor to finish too early
    try {
     Thread.sleep(3000);
    } catch (InterruptedException e) {
     e.printStackTrace();
    }
    this.value = value;
  }

  public void assertValue()  {
    if(value != value) System.out.println("Magic");
  }
}

I've added Thread.sleep(3000), to force thread to wait before object will be fully constructed.

public class Tests {

  private HolderContainer hc = new HolderContainer();

  class Initialization implements Runnable {
    public void run() {
      hc.init();
    }
  }

  class Checking implements Runnable {
    public void run() {
      hc.holder.assertValue();
    }
  }

  public void run() {
    new Thread(new Initialization()).start();
    new Thread(new Checking()).start();
  }
}

In example:

  1. first thread inits holder object
  2. second thread calls assertValue

Main Thread runs two threads:

  1. new Thread(new Initialization()).start(); It tooks 3 seconds to fully construct Holder object
  2. new Thread(new Checking()).start(); since Holder object still not constructed code will throw an NullPointerException

Therefore, it's impossible to emulate situation when field has default value.

My Questions:

  1. Author was wrong about this concurrency problem?
  2. Or It it impossible to emulate behaviour for fields default values?
Colicweed answered 2/6, 2018 at 10:18 Comment(12)
@VinceEmigh, I've tested it on Java 6 and Java 8Colicweed
@AxelH as I wrote: "new Thread(new Checking()).start(); since Holder object still not constructed code will throw an exception". By This I meant that we will have NPE.Colicweed
But is this Tests class from the book ? (sorry, skipped those two point)Bite
@AxelH, unfortunately no. Author just made states, and gave us two first classes: Holder and HolderContainer. So Tests is my class. With it I'm trying to check if author's states is right.Colicweed
Might need to check more test case then. Got it ;)Bite
@Bite Sorry don't get you questionColicweed
Possible duplicate of Improper publication of Java Object ReferenceIntrinsic
Sorry to say, I was unable to reproduce this myself. However, I did find another post asking about this exact situation. Sadly, I was unable to find any examples that reproduces the problem on that post. Interesting enough, the answer there states: "Under an x86 arch this is, from what I know, impossible to happen but may not be the case in others."Intrinsic
It's only possible if the thread in started from inside the constructor - see here. I was able to trigger the exception using the example from the link but only when I change the assertion to if (value != 42).Klump
The author is correct, but it may be quite difficult to get that assertion error. You could try these tests over and over in many threads at the same time with no sleeps, and even then it might not work, because the compiler can optimize the value != value check so that it's always false. Nonetheless, the author is correct. Many concurrency bugs only show up when you're unlucky. Also, @Klump is not correct when he says the problem is only possible if the thread is stared in the construtor. Other threads may not see writes in the same order that they are made.Transposition
Honestly, don't even think about "testing" for race conditions. You might want to try out system load testing and running randomised automatic, but don't think you are going to be write a test for specific race conditions. I had to write demonstrations race conditions that caused exploitable security vulnerabilities in the Java Class Library. It's really unpleasant and they are really bad tests. [1/2]Sodomite
Race condition "tests" rely upon very specific implementation details, take ages to run and unpleasant to write. (I used to get the demonstration to the point that it'd pick the bug up within about five minutes on the machine I happened to be using.) Not to mention tying up your very best programmers (ION, anybody got a job for a Java programmer in Edinburgh?). Honestly, automatic tools and not being "clever" is the way to go. [2/2]Sodomite
A
2

I tried to test the problem with the following code.

Test:

public class Test {
    public static boolean flag =true;
    public static HolderContainer hc=new HolderContainer();

    public static void main (String args[]){    
        new Thread(new Initialization()).start();
        new Thread(new Checking()).start();
    }
}

class Initialization implements Runnable {
    public void run() {
        while (Test.flag){
            Test.hc=new HolderContainer();
            Test.hc.init();
            }
    }
}

class Checking implements Runnable {
    public void run() {
        try{
            Test.hc.holder.assertValue();
        }
        catch (NullPointerException e) {
        }    
    }
}

Holder:

public class Holder {
    private int value;
        public Holder(int value) { 
        this.value = value;
    }

    public void assertValue() {
        if (value != value) {
            System.out.println("Magic");
            Test.flag=false;
        }
    }
}

class HolderContainer {
    public Holder holder;
    public void init() {
        holder = new Holder(42);  
    }
}

I never got the program to evaluate value!=value to true. I don't think this proves anything and didn't run it for more than a couple minutes, but I hope this will be a better starting point for a well designed test or at least help to figure out some possible flaws in the tests.

I tried to insert a sleep between Test.hc=new HolderContainer(); and Test.hc.init();, between public Holder holder; and public void init() { and after public void init() {.

I am also concerned that checking if a value is null or catching the NullPoiterException may affect the timing too much.

Please note that the currently accepted answer to Improper publication of Java Object Reference says this problem is probably impossible under an x86 architecture. It may also be JVM dependent.

Answerable answered 2/6, 2018 at 10:18 Comment(3)
My opinion - I am not very sure - is that the reason why it's not possible on x86 Arch is that there is a compiler optimization which replaces 'value != value' to false. EDIT - sorry, now I see that in the linked question a different aspect of reads and writes is being discussed.Spotless
I tried for a bit with "-Djava.compiler=NONE" and with "value != 42". Not sure this would help. Also just in case here is the bytecode for the comparision:0: aload_0 /1: getfield #7/4: aload_0/5: getfield #7/8: if_icmpeq 23. From what I got there isn't any simplification by the compiler.Answerable
@Spotless As far as I can see optimisation can have an impotant role. The original probelm seems to be about the specification not guaranteeing that this problem won't happen and not about this problem actually occurring in some specific implementation and reproducing this problem seems to be the only, arguably funny and/or useful, reason to keep this as a separate question and fiddling with the jvm optimisation seems legit for this task. It requires knowledge at a lot of different levels and I am not the more suited person for this and any help and/or splitting of the problen is welcomed.Answerable
R
0

Did not reproduce it with your code. Here is an example to emulate un-safe publication. The strategy is let one thread publication Holder and let another check its value.

class Holder {
    private volatile int value;

    public Holder(int value, HolderContainer container) {
        container.holder = this;  // publication this object when it is not initilized properly
        try {
            Thread.sleep(10);  
        } catch (Exception e) {

        }
        this.value = value; // set value
    }

    public int getValue() {
        return value;
    }
}

class HolderContainer {

    public Holder holder;

    public Holder getHolder() { 
        if (holder == null) { 
            holder = new Holder(42, this);
        }
        return holder;
    }
}


public class Tests {

    public static void main(String[] args) {
        for (int loop = 0; loop < 1000; loop++) {
            HolderContainer holderContainer = new HolderContainer();
            new Thread(() -> holderContainer.getHolder()).start();
            new Thread(() -> {
                Holder holder = holderContainer.getHolder();
                int value1 = holder.getValue();  // might get default value
                try {
                    Thread.sleep(10);
                } catch (Exception e) {

                }
                int value2 = holder.getValue(); // might get custom value
                if (value1 != value2) {
                    System.out.println(value1 + "--->" + value2);
                }
            }).start();
        }
    }

}
Rb answered 2/6, 2018 at 11:35 Comment(3)
I am not sure this example fits; here the order of the operations is inverted. The consistency issue from the book is about threads seeing operations in a different order. Here it is obviously possible to see value=0 in the reader thread even if the two threads agree on the order in which operations are executed.Armes
@Armes I can not say it 100% fits. I posted it because I think the point is about safe publication. If the community think this does not make sense, I will delete it.Rb
@NengLiu unsafe publication is 100% logical thing. My question is about case when Holder's super() (i.e. Object constructor) constructor have been already executed, but Holder constructor still not. In that case author states that we could see stale value = 0. And for me it's seems that is impossible to reproduce that behaviour.Colicweed
P
0

You are probably trying to simulate a concurrency scenario, which I believe is very hard to simulate using a couple of threads.

The following test-case which you have written is not correct at all is more likely to throw a NullPointerException.

public class Tests {

  private HolderContainer hc = new HolderContainer();

  class Initialization implements Runnable {
    public void run() {
      hc.init();
    }
  }

  class Checking implements Runnable {
    public void run() {
      hc.holder.assertValue();
    }
  }

  public void run() {
    new Thread(new Initialization()).start();  
    new Thread(new Checking()).start(); 
  }
}

What if your Checking Thread executes before Initialization one?? Also putting a sleep there simply means that executing thread will sleep and does tell you about the internal atomic operations being performed by then.

Patriciate answered 2/6, 2018 at 11:37 Comment(6)
My Question is about is it possible to get default and then non default value. As I mentioned in my question, my code give NullPoinnterExtecion which is 100% logical. But Author states that somehow it possible to get value != value case.Colicweed
Yes it is possible in a real world scenario where you have millions of requests are madePatriciate
And plz don't it literally (millions of request), All applications use pools for handling requestsPatriciate
So the question is how to reproduce it?Colicweed
Increase the number of threads accessing the same object. For the sake of testing, you can raise it to ~300-500 Also exceute it using terminal/cmd javac & java command in quick succession and write everything to a log file. No matter if any exceptions come. With that number of thread you may be able to reproduce itPatriciate
The book shows the expression value != value, which is the confusing part. I don't see how this answer explains that possibility. This answer simply regurgitates what the book said: "It's theoretically possible". You do suggest that his test is not complex enough, but once again, that doesn't bring us any closer to the answer, or being able to mock an environment that could reproduce this.Intrinsic
R
0

Sleeping 3 seconds before assigning the field in the constructor does not matter because for value != value to be true, the first read of value must produce a different result than the second one, which happens immediately after.

The Java Memory Model does not guarantee that values assigned to fields in constructors are visible to other threads after the constructor finishes. To have this guarantee, the field must be final.

Here's a program that produces the bug on x86. It must be run with the VM option: -XX:CompileCommand=dontinline,com/mypackage/Holder.getValue

package com.mypackage;

public class Test {
    public static void main(String[] args) {
        new Worker().start();
        int i = 1;
        while (true) {
            new Holder(i++);
        }
    }
}

class Holder {
    private int value;

    Holder(int value) {
        Worker.holder = this;
        this.value = value;
    }

    void assertSanity() {
        if (getValue() != getValue()) throw new AssertionError();
    }

    private int getValue() { return value; }
}

class Worker extends Thread {
    static Holder holder = new Holder(0);

    @Override
    public void run() {
        while (true) {
            holder.assertSanity();
        }
    }
}

By disallowing Holder#getValue() to be inlined, we prevent the two subsequent reads of value to be collapsed into a single one.

This optimization prevents the code in the book from producing the bug. However, the book author is still correct, since this optimization is not mandatory, so from the Java Memory Model perspective, the code is incorrect.

The assertSanity() method is equal to:

int snapshot1 = getValue();
                            // <--- window of vulnerability, where the observed value can change
                            //      if you chose to sleep 3 seconds, you would want to do it here
                            //      takes very little time, less than 1 nanosecond
int snapshot2 = getValue();
if (snapshot1 != snapshot2) throw new AssertionError();

So the first read of value could produce the default value of int which is 0 (called the stale value and assigned in the Object() constructor), and the second read could produce the value assigned in the Holder(int) constructor. This would happen if for example the value assigned in the constructor were propagated to the thread calling assertSanity() in the exact moment between the two loads of value (window of vulnerability).

The same would happen if we delayed the second read in some other way, like:

int snapshot1 = this.value;
Thread.interrupted();
int snapshot2 = this.value;
if (snapshot1 != snapshot2) throw new AssertionError();
Ranite answered 27/7, 2020 at 5:20 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.