[concurrency-interest] synchronized constructors

Yuval Shavit yshavit at akiban.com
Thu Dec 15 16:59:45 EST 2011


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/ee26bc39/attachment.html>


More information about the Concurrency-interest mailing list