<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Per, your analysis seems to be right on the money.<br>
    <br>
    After initializing the variable and putting in asserts in join/leave<br>
    and removing the asserts (which did indeed get hit) in
    synchronize/desynchronize<br>
    it passes jprt.<br>
    <br>
    Here's the latest webrev for review:<br>
    <br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jprovino/7006810/webrev.03">http://cr.openjdk.java.net/~jprovino/7006810/webrev.03</a><br>
    <br>
    Thanks for all the reviews.<br>
    <br>
    joe<br>
    <br>
    <div class="moz-cite-prefix">On 4/9/2015 4:00 PM, Srinivas
      Ramakrishna wrote:<br>
    </div>
    <blockquote
cite="mid:CABzyjymQoKcLbGsT7xGB-nfgm=X+iCcKWVeASwSFsYgdO=mX-w@mail.gmail.com"
      type="cite">
      <div dir="ltr">
        <div><br>
          <div class="gmail_extra"><br>
            <div class="gmail_quote">On Thu, Apr 9, 2015 at 2:00 AM, Per
              Liden <span dir="ltr"><<a moz-do-not-send="true"
                  href="mailto:per.liden@oracle.com" target="_blank">per.liden@oracle.com</a>></span>
              wrote:<br>
              <blockquote class="gmail_quote" style="margin:0 0 0
                .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi
                Joe/Ramki,<span class=""><br>
                  <br>
                  On 2015-04-09 09:58, Srinivas Ramakrishna wrote:<br>
                </span>
                <blockquote class="gmail_quote" style="margin:0 0 0
                  .8ex;border-left:1px #ccc solid;padding-left:1ex">
                  Hi Joe --<br>
                  <br>
                  <span class="">
                    On Wed, Apr 8, 2015 at 4:27 PM, Joseph Provino<br>
                  </span>
                  <div>
                    <div class="h5">
                      <<a moz-do-not-send="true"
                        href="mailto:joseph.provino@oracle.com"
                        target="_blank">joseph.provino@oracle.com</a>
                      <mailto:<a moz-do-not-send="true"
                        href="mailto:joseph.provino@oracle.com"
                        target="_blank">joseph.provino@oracle.com</a>>>
                      wrote:<br>
                      <br>
                      ...<br>
                          On 4/6/2015 2:20 PM, Srinivas Ramakrishna
                      wrote:<br>
                      <blockquote class="gmail_quote" style="margin:0 0
                        0 .8ex;border-left:1px #ccc
                        solid;padding-left:1ex">...</blockquote>
                      <blockquote class="gmail_quote" style="margin:0 0
                        0 .8ex;border-left:1px #ccc
                        solid;padding-left:1ex">
                            Do we allow threads to recursively enter the
                        suspendible thread<br>
                            set in nested fashion? I'm guessing we
                        don't, in which case, you would<br>
                            use this information local to the thread to
                        prevent/forbid such<br>
                            recursive entries or exits when it isn't
                        part of the set, by asserting<br>
                            that the bit isn't set when a thread joins
                        the set and never clear<br>
                            before a thread leaves the set. I understand
                        that there's a scoped<br>
                            join/leave abstraction via
                        SuspendibleThreadSetJoiner, but there<br>
                            also seem to be a bunch of naked join/leave
                        calls, so the<br>
                            extra checking would likely be worthwhile.<br>
                      </blockquote>
                          This is a good question.  If I put in checks
                      in join / leave<br>
                      <br>
                          void SuspendibleThreadSet::join() {<br>
                             assert(Thread::current()->is_suspendible_thread()
                      == false,<br>
                               "Thread already joined");<br>
                      <br>
                          void SuspendibleThreadSet::leave() {<br>
                             //assert(Thread::current()->is_suspendible_thread(),<br>
                             //  "Thread not joined");<br>
                      <br>
                          jprt hits the assert in join.<br>
                      <br>
                          It doesn't seem right to call join if a thread
                      is already joined.<br>
                          Would it be correct to just return if trying
                      to rejoin or trying to<br>
                          leave when not joined?<br>
                      <br>
                      <br>
                      That would imply (along with the use of the scoped
                      join/leave<br>
                      abstraction) that nested joins/leaves are allowed.<br>
                      I'd suggest then, that a thread keep track of the
                      number of times it<br>
                      joined and left by using a full-fledged<br>
                      counter instead of a boolean bit.<br>
                      Then your asserts would check that it wasn't
                      trying to leave when its<br>
                      count was 0.<br>
                    </div>
                  </div>
                </blockquote>
                <br>
                Even if the STS allows nested joins (or rather, today it
                doesn't check for it), to the best of my knowledge we
                don't have any such threads. Joe, I suggest that you
                look at the threads using STS (not that many) to confirm
                that this is the case. I don't think we should allow it,
                and therefor I think the thread state should be kept as
                a bool rather than a counter. As I mentioned in my other
                reply, there's a bug here. Since _suspendible_thread is
                never properly initialized it might look like we have
                already joined, when in fact we have just forgot to
                initialize it. I suspect that's the reason why it might
                look like we have nested joins.<br>
                <br>
              </blockquote>
              <div><br>
              </div>
              <div>Ah, OK. Hopefully Joe's testing after fixing the
                initialization confirmed that that was what was
                triggering the assert. I agree that it makes sense to
                keep it simple and disallow nested joins, if possible.
                (Haven't looked at the exit protocol carefully, but I
                will, as well as looking at the newer webrev.)</div>
              <div><br>
              </div>
              <div>-- ramki </div>
            </div>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>