[concurrency-interest] synchronized constructors

David Holmes davidcholmes at aapt.net.au
Thu Dec 15 17:44:41 EST 2011


No I take that back. final field semantics are more subtle than that. I'm
unsure if writing to a final would suffice.

David
  -----Original Message-----
  From: David Holmes [mailto:davidcholmes at aapt.net.au]
  Sent: Friday, 16 December 2011 8:42 AM
  To: Yuval Shavit
  Cc: Nathan Reynolds; concurrency-interest
  Subject: RE: [concurrency-interest] synchronized constructors


  No - final fields have special semantics in the JMM. See "Final Fields" at

  http://g.oswego.edu/dl/jmm/cookbook.html

  David
    -----Original Message-----
    From: Yuval Shavit [mailto:yshavit at akiban.com]
    Sent: Friday, 16 December 2011 8:36 AM
    To: dholmes at ieee.org
    Cc: Nathan Reynolds; concurrency-interest
    Subject: Re: [concurrency-interest] synchronized constructors


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


More information about the Concurrency-interest mailing list