Java Memory Model: Is it safe to create a cyclical

2019-04-04 09:48发布

问题:

Can somebody who understand the Java Memory Model better than me confirm my understanding that the following code is correctly synchronized?

class Foo {
    private final Bar bar;

    Foo() {
        this.bar = new Bar(this);
    }
}

class Bar {
    private final Foo foo;

    Bar(Foo foo) {
        this.foo = foo;
    }
}

I understand that this code is correct but I haven't worked through the whole happens-before math. I did find two informal quotations that suggest this is lawful, though I'm a bit wary of completely relying on them:

The usage model for final fields is a simple one: Set the final fields for an object in that object's constructor; and 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. If this is followed, then when the object is seen by another thread, that thread will always see the correctly constructed version of that object's final fields. It will also see versions of any object or array referenced by those final fields that are at least as up-to-date as the final fields are. [The Java® Language Specification: Java SE 7 Edition, section 17.5]

Another reference:

What does it mean for an object to be properly constructed? It simply means that no reference to the object being constructed is allowed to "escape" during construction. (See Safe Construction Techniques for examples.) In other words, do not place a reference to the object being constructed anywhere where another thread might be able to see it; do not assign it to a static field, do not register it as a listener with any other object, and so on. These tasks should be done after the constructor completes, not in the constructor. [JSR 133 (Java Memory Model) FAQ, "How do final fields work under the new JMM?"]

回答1:

Yes, it is safe. Your code does not introduce a data race. Hence, it is synchronized correctly. All objects of both classes will always be visible in their fully initialized state to any thread that is accessing the objects.

For your example, this is quite straight-forward to derive formally:

  1. For the thread that is constructing the threads, all observed field values need to be consistent with program order. For this intra-thread consistency, when constructing Bar, the handed Foo value is observed correctly and never null. (This might seem trivial but a memory model also regulates "single threaded" memory orderings.)

  2. For any thread that is getting hold of a Foo instance, its referenced Bar value can only be read via the final field. This introduces a dereference ordering between reading of the address of the Foo object and the dereferencing of the object's field pointing to the Bar instance.

  3. If another thread is therefore capable of observing the Foo instance altogether (in formal terms, there exists a memory chain), this thread is guaranteed to observe this Foo fully constructed, meaning that its Bar field contains a fully initialized value.

Note that it does not even matter that the Bar instance's field is itself final if the instance can only be read via Foo. Adding the modifier does not hurt and better documents the intentions, so you should add it. But, memory-model-wise, you would be okay even without it.

Note that the JSR-133 cookbook that you quoted is only describing an implementation of the memory model rather than then memory model itself. In many points, it is too strict. One day, the OpenJDK might no longer align with this implementation and rather implement a less strict model that still fulfills the formal requirements. Never code against an implementation, always code against the specification! For example, do not rely on a memory barrier being placed after the constructor, which is how HotSpot more or less implements it. These things are not guaranteed to stay and might even differ for different hardware architectures.

The quoted rule that you should never let a this reference escape from a constructor is also too narrow a view on the problem. You should not let it escape to another thread. If you would, for example, hand it to a virtually dispatched method, you could not longer control where the instance would end up. This is therefore a very bad practice! However, constructors are not dispatched virtually and you can safely create circular references in the manner you depicted. (I assume that you are in control of Bar and its future changes. In a shared code base, you should document tightly that the constructor of Bar must not let the reference slip out.)



回答2:

Immutable Objects (with only final fields) are only "threadsafe" after they are properly constructed, meaning their constructor has completed. (The VM probably accomplishes this by a memory barrier after the constructor of such objects)

Lets see how to make your example surely unsafe:

  • If the Bar-Constructor would store a this-reference where another thread could see it, this would be unsafe because Bar isnt constructed yet.
  • If the Bar-Constructor would store a foo-reference where another thread could see it, this would be unsafe because foo isnt constructed yet.
  • If the Bar-Constructor would read some foo-fields, then (depending on the order of initialization inside the Foo-constructor) these fields would always be uninitialized. Thats not a threadsafety-problem, just an effect of the order of initialization. (Calling a virtual method inside a constructor has the same issues)

References to immutable Objects (only final fields) which are created by a new-expression are always safe to access (no uninitialized fields visible). But the Objects referenced in these final fields may show uninitialized values if these references were obtained by a constructor giving away its this-reference.

As Assylias already wrote: Because in your example the constructors stored no references to where another thread could see them, your example is "threadsafe". The created Foo-Object can safely be given other threads.