[concurrency-interest] synchronized constructors

Yuval Shavit yshavit at akiban.com
Thu Dec 15 17:36:29 EST 2011


Wouldn't the final field only establish that reads to *that field* see at
least the state it was in at end of construction? That is, if you had two
fields:

final int a;
int b;

And your constructor were:
    a = 1;
    b = a;

Then, in the absence of correct publication, couldn't another thread still
see a = 1, b = 0?

On Thu, Dec 15, 2011 at 5:25 PM, David Holmes <davidcholmes at aapt.net.au>wrote:

> **
> final field yes. Write to a volatile no.
>
> The volatile only works if the other thread reads the volatile and sees
> the written value.
>
> David
>
> -----Original Message-----
> *From:* concurrency-interest-bounces at cs.oswego.edu [mailto:
> concurrency-interest-bounces at cs.oswego.edu]*On Behalf Of *Nathan Reynolds
> *Sent:* Friday, 16 December 2011 8:20 AM
> *To:* Yuval Shavit
> *Cc:* concurrency-interest
> *Subject:* Re: [concurrency-interest] synchronized constructors
>
> > it seems to me that if all of the constructor had been wrapped in a
> synchronized (this) {...}
>
> One could also add a final field in the class or have the constructor
> write to a volatile at the end of the method.  Both of these would add a
> happens-before.
>
> Nathan Reynolds<http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>| Consulting Member of Technical Staff |
> 602.333.9091
> Oracle PSR Engineering <http://psr.us.oracle.com/> | Server Technology
>
> On 12/15/2011 2:59 PM, Yuval Shavit wrote:
>
> Oh, I totally understand why it's a bad idea -- and I was about to reply
> to your reply to Ruslan, saying that my version was basically a poor man's
> double-checked lock. It's actually not even a real DCL, because that
> pattern at least tries to ensure that only one thread will instantiate the
> object, whereas my code will happily create multiple instances, each living
> in its own place on some CPU's cache. The hypothetical author of that code
> might even say that's intentional, because he wants to shard or something.
> Like I said, I fully understand it's a bad pattern. On the other hand, it
> seems to me that if all of the constructor had been wrapped in a
> synchronized (this) {...}, this broken sharding/DCL would work: either you
> instantiate the object yourself (and thus see it fully instantiated) or
> you'll hit a synchronized(this) block on toString(), which will establish
> that the constructor's synchronized(this) happened-before toString's.
>
> On Thu, Dec 15, 2011 at 4:52 PM, Nathan Reynolds <
> nathan.reynolds at oracle.com> wrote:
>
>> This smells like double-checked locking.  Without a volatile or
>> synchronized, then this is definitely not going to work correctly and is
>> not thread-safe.  Look up double-checked locking on the web to see why it
>> is not thread-safe.
>>
>>
>> Nathan Reynolds<http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>| Consulting Member of Technical Staff |
>> 602.333.9091
>> Oracle PSR Engineering <http://psr.us.oracle.com/> | Server Technology
>>
>>  On 12/15/2011 2:43 PM, Yuval Shavit wrote:
>>
>> So is the assumption that safe initial publication to other threads is
>> always required, and thread safety guarantees only kick in once that's been
>> done?
>>
>> I could imagine someone assigning their MyPoint to a non-volatile static
>> and creating a new instance if that static is null:
>>
>> public static MyPoint myPoint;
>>
>> MyPoint getAnyOldPoint() {
>>     if (myPoint == null)
>>         myPoint = new MyPoint();
>>     return myPoint;
>> }
>>
>> The person who writes that code (not me! :)  ) says, "I don't care which
>> MyPoint I get -- I don't care if it's the same one another thread sees or
>> not. All I care is that there is one, and that it's thread-safe. And I got
>> one (didn't have to instantiate it in this method), but it wasn't thread
>> safe: I saw "(0,-1)" even though no thread could have possibly set such a
>> state. I must have seen the initial state of a MyPoint, but it wasn't
>> thread safe."
>>
>> Now, I would love to tell them "sure it's thread safe, you're just doing
>> stupid things." But would they be wrong in telling me that no, my class is
>> only *partially* thread safe?
>>
>> Yuval
>>
>>
>>  On Thu, Dec 15, 2011 at 4:30 PM, Nathan Reynolds <
>> nathan.reynolds at oracle.com> wrote:
>>
>>> How is the MyPoint instance going to become shared with another thread?
>>> The thread calling the constructor has to execute some sort of
>>> synchronization to share the reference to the MyPoint instance.  The
>>> synchronization will cause a happens-before edge to be created.  For
>>> example, this code assigns a volatile field.  Writing to the volatile will
>>> ensure the proper values of x and y will be visible.
>>>
>>> public volatile MyPoint point = new MyPoint();
>>>
>>> Nathan Reynolds<http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>| Consulting Member of Technical Staff |
>>> 602.333.9091
>>> Oracle PSR Engineering <http://psr.us.oracle.com/> | Server Technology
>>>
>>> On 12/15/2011 2:10 PM, Yuval Shavit wrote:
>>>
>>>  Hi all,
>>>
>>> This came up a few days ago on stackoverflow.com -- I forget the exact
>>> post, but someone referenced the JLS section concerning why constructors
>>> can't be synchronized:
>>>
>>> There is no practical need for a constructor to be synchronized, because
>>>> it would lock the object under construction, which is normally not made
>>>> available to other threads until all constructors for the object have
>>>> completed their work.
>>>
>>>
>>> If synchronization were only about mutual exclusion, this would make
>>> sense; but it also has memory visibility guarantees. For instance, say I
>>> have this really simple class:
>>>
>>>      public class MyPoint {
>>>         private int x;
>>>         private int y;
>>>
>>>         public MyPoint() {
>>>             x = -1; // init to (-1,-1) for some reason
>>>             y = -1;
>>>         }
>>>
>>>         public synchronized void setX(int x, int y) {
>>>             this.x = x;
>>>             this.y = y;
>>>         }
>>>
>>>         @Override
>>>         public synchronized String toString() {
>>>             return "(" + x + ", " + y + ")";
>>>         }
>>>     }
>>>
>>> There's a thread safety issue there, right? There was no synchronization
>>> during construction time, and thus no happens-before between ctor ending
>>> and toString() starting, so with reordering etc, you could observe the
>>> initial state as "(0, 0)", "(-1, 0)", "(-1, -1)" or "(-1, 0)".  You could
>>> get around this by calling setX from the constructor, but if setX is
>>> non-final, that has its own issues. You could also put the x and y
>>> initialization within a synchronized (this) { ... } -- but at that point,
>>> why not just allow the constructor to be synchronized?
>>>
>>> Thanks,
>>> Yuval
>>>
>>>
>>> _______________________________________________
>>> Concurrency-interest mailing listConcurrency-interest at cs.oswego.eduhttp://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20111215/c34a618b/attachment.html>


More information about the Concurrency-interest mailing list