8

say we have this

// This is trivially immutable.
public class Foo {
    private String bar;
    public Foo(String bar) {
        this.bar = bar;
    }
    public String getBar() {
        return bar;
    }
}

What makes this thread unsafe ? Following on from this question.

Community
  • 1
  • 1
NimChimpsky
  • 46,453
  • 60
  • 198
  • 311
  • 6
    Isn't the answer to this question contained within the answer to that question? In particular, read http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5-110 – Jon Skeet Apr 17 '13 at 14:07
  • 1
    Saw your comment to my question and iam curious about answer – DRastislav Apr 17 '13 at 14:08
  • Possible duplicate : http://stackoverflow.com/questions/15072578/is-string-get-set-threadsafe – AllTooSir Apr 17 '13 at 14:09
  • 7
    @NoobUnChained definitely not a duplicate of that. No setter here. – NimChimpsky Apr 17 '13 at 14:10
  • @JonSkeet To me the other question described how final prevents data race issues, but didn't get into what could go wrong - threads seeing partially initialised objects due to JVM memory model reordering, etc – Carl Pritchett Oct 31 '13 at 21:20
  • @CarlPritchett: He explicitly gave an example of that, at the end of the answer... – Jon Skeet Oct 31 '13 at 21:38

4 Answers4

12

Foo is thread safe once it has been safely published. For example, this program could print "unsafe" (it probably won't using a combination of hotspot/x86) - if you make bar final it can't happen:

public class UnsafePublication {

    static Foo foo;

    public static void main(String[] args) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (foo == null) {}
                if (!"abc".equals(foo.getBar())) System.out.println("unsafe");
            }
        }).start();

        new Thread(new Runnable() {
            @Override
            public void run() {
                foo = new Foo("abc");
            }
        }).start();
    }
}
assylias
  • 321,522
  • 82
  • 660
  • 783
  • What about `foo.getBar()` if `String bar` hasn't been assigned yet ? – AllTooSir Apr 17 '13 at 14:25
  • @assylias but `foo.getBar()` may still return `null`, doesn't it? – Adam Dyga Apr 17 '13 at 14:26
  • @Eugene I think as per [usage model for final fields](http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5-110), merely making the fields as final won't guarantee safe publishing rather we still have to make sure that do not write a reference to the object being constructed in a place where another thread can see it before the object's constructor is finished. So here if we make bar field final then as thread2 updates foo after Foo's constructor finishes thus thread1 method will be guaranteed to see properly initialized value for bar i.e. foo.getBar() will never return null. – sactiw Nov 25 '13 at 13:21
7

Due to JVM optimization, you can never assume that operations are executed in the order they are written, unless it matters for the same thread. So when you call the constructor and then pass a reference to the resulting object to another thread, the JVM might not actually write the value of foo.bar before it is needed within the same thread.

That means that in a multithreaded environment, the getBar method could be called before the value in the constructor was written to it.

Philipp
  • 67,764
  • 9
  • 118
  • 153
7

Most probably you've got your answer by now, but just to make sure I wanted to add my explanation also.

In order for an object (for your case) to be thread-safe, it must:

  • Be immutable
  • Be safely published

Immutable - you made it so. There is no way to modify bar once it has been set. Pretty obvious here.

Safely published. As per the example, the code is not safely published. Because bar is not final the compiler is free to reorder it as it finds appropriate. The compiler could publish(write to main memory) the reference to Foo instance, before the write to bar. That would mean that bar is null. So, first the reference to Foo is written to main memory, then the write to bar happens. In between these two events another thread can see the stale bar as being null.

If you add final to it, the JMM will guarantee that:

the values of final fields are guaranteed to be visible to other threads accessing the constructed object.

Or, final field prevents reordering. Thus making that variable final will ensure thread safety.

Eugene
  • 117,005
  • 15
  • 201
  • 306
1

From the link posted in comments:

class FinalFieldExample { 
    final int x;
    int y; 
    static FinalFieldExample f;

    public FinalFieldExample() {
        x = 3; 
        y = 4; 
    } 

    static void writer() {
        f = new FinalFieldExample();
    } 

    static void reader() {
        if (f != null) {
            int i = f.x;  // guaranteed to see 3  
            int j = f.y;  // could see 0
        } 
    } 
}

One thread may call writer() and and another thread may call reader(). The if condition in reader() could evaluate to true, but becuase y is not final the object initalizion may not have completely finished (so the object has not been safely published yet), and thus int j = 0 could happen as it has not been initialized.

NimChimpsky
  • 46,453
  • 60
  • 198
  • 311
  • so.. a final assignment is atomic? – Eugene Apr 18 '13 at 09:01
  • @Eugene, every assignment is atomic in "recent" JVMs (including 64-bit fields). – Bruno Reis Apr 23 '13 at 14:32
  • @BrunoReis true. What I meant is this: if y is final also(in the example above), does that mean that both variables are set to the corresponding values in an atomic way? Or make it even simpler: does this mean that the constructor is atomic aka thread safe? No other thread ca see a stale value of x or/and y? – Eugene Apr 24 '13 at 06:24
  • 1
    @Eugene: if the object is published only after the constructor returns (ie, you don't pass `this` to anything external from inside the constructor), then all the final fields are guaranteed to be seen initialized by every thread that gets a reference to that object. – Bruno Reis Apr 24 '13 at 15:13