[concurrency-interest] synchronized constructors

David Holmes davidcholmes at aapt.net.au
Thu Dec 15 17:21:02 EST 2011


Generally speaking the thread-safety of a class pertains to the use of an
instance of that class once it has been shared between threads. The sharing,
in general, requires safe-publication, which is something normally under the
control of the code doing the sharing not the code of the object that was
shared. Sometime a class needs to add some protection against
unsafe-publication that might violate important semantic guarantees of the
object - like sharing Strings in a way that might make the string appear to
have different values hence String uses final fields and we have the JMM's
final field semantics.

It is rare that you need to synchronize a constructor, or that having a
synchronized constructor addresses your needs. But in the case where it does
then you can use a synchronized block within the constructor.

I would not be surprised that when the prohibition against synchronizing a
constructor was created, visibility and safe-publication were not
considerations.

Cheers,
David Holmes
  -----Original Message-----
  From: concurrency-interest-bounces at cs.oswego.edu
[mailto:concurrency-interest-bounces at cs.oswego.edu]On Behalf Of Yuval Shavit
  Sent: Friday, 16 December 2011 8:00 AM
  To: Nathan Reynolds
  Cc: concurrency-interest
  Subject: Re: [concurrency-interest] synchronized constructors


  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 | Consulting Member of Technical Staff | 602.333.9091
    Oracle PSR Engineering | 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 | Consulting Member of Technical Staff |
602.333.9091
        Oracle PSR Engineering | 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 list
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/20111216/ebb16e8a/attachment.html>


More information about the Concurrency-interest mailing list