[concurrency-interest] synchronized constructors

Nathan Reynolds nathan.reynolds at oracle.com
Thu Dec 15 17:20:11 EST 2011


 > 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 <mailto: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 <tel: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 <mailto: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
>>         <tel: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
>>>         <http://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 list
>>>         Concurrency-interest at cs.oswego.edu  <mailto:Concurrency-interest at cs.oswego.edu>
>>>         http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20111215/0b4bb739/attachment-0001.html>


More information about the Concurrency-interest mailing list