Question

Referring to my earlier question on incompletely constructed objects, I have a second question. As Jon Skeet pointed out, there's an implicit memory barrier in the end of a constructor that makes sure that final fields are visible to all threads. But what if a constructor calls another constructor; is there such a memory barrier in the end of each of them, or only in the end of the one that got called in the first place? That is, when the "wrong" solution is:

public class ThisEscape {
    public ThisEscape(EventSource source) {
        source.registerListener(
            new EventListener() {
                public void onEvent(Event e) {
                    doSomething(e);
                }
            });
    }
}

And the correct one would be a factory method version:

public class SafeListener {
    private final EventListener listener;

    private SafeListener() {
        listener = new EventListener() {
            public void onEvent(Event e) {
                doSomething(e);
            }
        }
    }

    public static SafeListener newInstance(EventSource source) {
        SafeListener safe = new SafeListener();
        source.registerListener(safe.listener);
        return safe;
    }
}

Would the following work too, or not?

public class MyListener {
    private final EventListener listener;

    private MyListener() {
        listener = new EventListener() {
            public void onEvent(Event e) {
                doSomething(e);
            }
        }
    }

    public MyListener(EventSource source) {
        this();
        source.register(listener);
    }
}

Update: The essential question is that is this() guaranteed to actually call the private constructor above (in which case there would be the barrier where intended and everything would be safe), or is it possible that the private constructor gets inlined into the public one as an optimization to save one memory barrier (in which case there wouldn't be a barrier until in the end of the public constructor)?

Are the rules of this() defined precisely somewhere? If not, then I think we must assume that inlining chained constructors is allowed, and probably some JVMs or maybe even javacs are doing it.

Was it helpful?

Solution

I think it is safe as java memory model states that:

Let o be an object, and c be a constructor for o in which a final field f is written. A freeze action on final field f of o takes place when c exits, either normally or abruptly. Note that if one constructor invokes another constructor, and the invoked constructor sets a final field, the freeze for the final field takes place at the end of the invoked constructor.

OTHER TIPS

An object is considered to be completely initialized when its constructor finishes.

This applies also for chained constructors.

If you have to register in the constructor define the listener as a static inner class. This is safe.

Your second version is not correct, because it is allowing the 'this' reference to escape from the construction process. Having 'this' escape invalidates the initialization safety guarantees that give final fields their safety.

To address the implicit question, the barrier at the end of construction only happens at the very end of object construction. The intuition one reader offered about inlining is a useful one; from the perspective of the Java Memory Model, method boundaries do not exist.

EDIT After the comment that suggested the compiler inlining the private constructor (I had not thought of that optimization) chances are that the code will be unsafe. And the worst part of unsafe multithreaded code is that is seems to work, so you are better off avoiding it completely. If you want to play different tricks (you do really want to avoid the factory for some reason) consider adding a wrapper to guarantee the coherence of data in the internal implementation object and register in the external object.


My guess is that it will be fragile but ok. The compiler cannot know whether the internal constructor will be called only from within other constructors or not, so it has to make sure that the result would be correct for code calling only the internal constructor, so whatever mechanism it uses (memory barrier?) has to be in place there.

I would guess that the compiler would add the memory barrier at the end of each and every constructor. The problem is still there: you are passing the this reference to other code (possibly other threads) before it is fully constructed --that is bad--, but if the only ´construction´ that is left is registering the listener, then the object state is as stable as it will ever be.

The solution is fragile in that some other day, you or some other programmer may need to add another member to the object and may forget that the chained constructors is a concurrency trick and may decide to initialize the field in the public constructor, and in doing so will add a hard to detect potential data race in your application, so I would try to avoid that construct.

BTW: The guessed safety may be wrong. I don't know how complex/smart the compiler is, and whether the memory barrier (or the like) is something it could try to optimize away... since the constructor is private the compiler does have enough information to know that it is only called from other constructors, and that is enough information to determine that the synchronization mechanism is not necessary in the internal constructor...

Escaping object reference in c-tor can publish an incompletely constructed object. This is true even if the publication is the last statement in the constructor.

Your SafeListener might not behave ok in a concurrent environment, even if c-tor inlining is performed (which I think it's not - think about creating objects using reflection by accessing private c-tor).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top