[concurrency-interest] final field/constructor visibility (cf. CyclicBarrier)

Zhong Yu zhong.j.yu at gmail.com
Sun Sep 28 21:23:39 EDT 2014


Thread 1

synchronized(v1) { ... }  // [s1]
unsafeFoo = v1;  // [w]

Thread 2

v2 = unsafeFoo; // [r]
if(v2!=null)
    synchronized(v2) { ... }  // [s2]

If [s2] is ordered before [s1], then [r] happens-before [w]; at the
same time, [r] observes [w]. This is not allowed in JMM. See
http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4.5-500

Zhong Yu
bayou.io



On Sun, Sep 28, 2014 at 6:52 PM, Yuval Shavit <yankee.sierra at gmail.com> wrote:
> I had forgotten about that email, but now that I'm re-reading it, I don't
> understand it. Or rather, I understand its conclusions but not its initial
> assertion. Specifically, *why* is publication-via-superclass the only way to
> "beat" a synchronized constructor?
>
> If the code is:
>
>     Foo(int i) {
>         synchronized (this) {
>             this.i = i;
>         }
>     }
>
>     synchronized int get() {
>         return i;
>     }
>
>     static Foo unsafeFoo;
>
>     static void installFoo() {
>         unsafeFoo = new Foo(1);
>     }
>
> ... then the only HB edges we've established are that get() happens-before
> <init> OR <init> happens-before get. There are no other HB edges I see, and
> in particular there's no HB between <init> and the write to unsafeFoo in
> installFoo, nor is there a HB between a write to unsafeFoo and a read from
> it.
>
> So, if another thread reads a non-null unsafeFoo and invokes get() on it, I
> don't see what guarantees that the write to this.i in the constructor
> happens-before the read to this.i in get(). One of them must formally
> happen-before the other, but that's the only guarantee I see here.
>
> Apologies if I've turned this thread into a zombie of a 2.5-year-old thread,
> but I think the original question in this thread basically boils down to the
> same issue.
>
> On Sun, Sep 28, 2014 at 6:30 PM, Zhong Yu <zhong.j.yu at gmail.com> wrote:
>>
>> On Sun, Sep 28, 2014 at 10:43 AM, Yuval Shavit <yankee.sierra at gmail.com>
>> wrote:
>> > This is somewhat related to a discussion I started here on synchronized
>> > constructors (specifically, why they're not allowed). In particular:
>> >
>> > http://cs.oswego.edu/pipermail/concurrency-interest/2011-December/008634.html
>>
>> The conclusion of that thread is that synchronized constructor *works*
>> even with unsafe publication. See
>>
>> http://cs.oswego.edu/pipermail/concurrency-interest/2011-December/008642.html
>>
>> I made the opposite and wrong assertion in that thread, which
>> fortunately was corrected by others. Sorry about that.
>>
>> thurstonn's code with locking inside constructor should work as he
>> expected.
>>
>> Zhong Yu
>> bayou.io
>>
>>
>> >
>> > The short of it is that your code is _still_ not safe in the face of
>> > unsafe
>> > publication. Yes, you establish HB relationships between the lock/unlock
>> > in
>> > the constructor and other lock/unlocks -- but you still haven't
>> > guaranteed
>> > that the constructor is seen first, as un-intuitive as that sounds. Some
>> > thread could see (for instance) a getter invoked before the constructor.
>> > In
>> > that case, the default value happens-before the getter, which
>> > happens-before
>> > the constructor. Clearly, that's not helpful, but it's valid according
>> > to a
>> > strict reading of the JMM.
>> >
>> > To be safe even in the face of unsafe publication, you'd need to not
>> > just
>> > establish just any old HB edge, but more precisely to ensure that the
>> > constructor happens-before everything else. One way to do that could be:
>> >
>> >     volatile boolean ctorFinished;
>> >
>> >     CyclicBarrier(int parties) {
>> >         // lock/unlock, set this.count and this.parties...
>> >         ctorFinished = true;
>> >     }
>> >
>> >     int getCount() {
>> >         while (!ctorFinished) {} // spin, wait for the ctor to finish on
>> > the
>> > instantiating thread
>> >         this.lock.lock();
>> >         // etc
>> >     }
>> >
>> >
>> >
>> > On Sun, Sep 28, 2014 at 3:18 AM, thurstonn
>> > <thurston at nomagicsoftware.com>
>> > wrote:
>> >>
>> >> But why not as I referenced?
>> >>
>> >> <code>
>> >> CyclicBarrier(int parties)
>> >>     this.lock.lock();
>> >>     this.count = parties;
>> >>     this.lock.unlock();
>> >>     this.parties = parties;
>> >>
>> >> </code>
>> >>
>> >>
>> >> Is it just somehow unappealing/ungainly (not often you see self-lock
>> >> acquisition in a constructor - can't say I like it that much)?
>> >>
>> >> Of course you're right about safe publication,and I should have
>> >> referenced
>> >> the fact that it's only unsafe in the face of improper publication.
>> >>
>> >> Still I think it's valuable to have j.u.c objects be "either null or
>> >> multi-thread safe" where possible (e.g. ReentrantLock)
>> >>
>> >>
>> >>
>> >> --
>> >> View this message in context:
>> >>
>> >> http://jsr166-concurrency.10961.n7.nabble.com/final-field-constructor-visibility-cf-CyclicBarrier-tp11306p11309.html
>> >> Sent from the JSR166 Concurrency mailing list archive at Nabble.com.
>> >> _______________________________________________
>> >> Concurrency-interest mailing list
>> >> Concurrency-interest at cs.oswego.edu
>> >> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >
>> >
>> >
>> > _______________________________________________
>> > Concurrency-interest mailing list
>> > Concurrency-interest at cs.oswego.edu
>> > http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>> >
>
>


More information about the Concurrency-interest mailing list