<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 27/06/2019 20:01, Brian Burkhalter wrote:<br>
    <blockquote type="cite"
      cite="mid:8B15EB6A-2E66-42D9-8277-C25FB9279837@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode:
        space; line-break: after-white-space;" class="">
        <div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode:
          space; line-break: after-white-space;" class=""><a
            href="https://bugs.openjdk.java.net/browse/JDK-8184157"
            class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8184157</a></div>
        <div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode:
          space; line-break: after-white-space;" class=""><a
            href="http://cr.openjdk.java.net/~bpb/8184157/webrev.01/"
            class="" moz-do-not-send="true">http://cr.openjdk.java.net/~bpb/8184157/webrev.01/</a><br
            class="">
          <div class=""><br class="">
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    I think the analysis is good and the approach to consistently leave
    the release of the OVERLAPPED structure to when the completion event
    is posted is good.<br>
    <br>
    One thing that isn't immediately clear from the changes is whether
    it introduces a memory leak when there is an error initiating the
    I/O operation or where there is an async close at around the time
    that the I/O operation is initiated. I would need to study these
    cases further to see if there are issues.<br>
    <br>
    The Windows implementations of AsynchronousSocketChannel and
    AsynchronousServerSocketChannel also remove and free the OVERLAPPED
    structure then the socket operation completes immediately. Will they
    also need attention?<br>
    <br>
    The test will need cleanup, maybe rename it to
    LockReadWriteStressTest or something that is closer to what it
    actually does. It can use try-with-resources to avoid leaking
    resources when it fails. I assume "f" and "l" will be renamed as
    they are unusual variables names in this context. Also the buffer
    usages is unusual and I can assume can be cleaned up too.<br>
    <br>
    -Alan.<br>
  </body>
</html>