The change looks okay to me. Though the comment about the -1 case also applies if timeout is 0. The behavior is still okay in that case, but the comment should acknowledge that,
however unlikely it is to occur.

- Michael.

On 10/03/2020 16:37, Daniel Fuchs wrote:
Hi Vyom,

I have sent your proposed fix to our test system and observed no
regression. I agree your proposed changes seem to address the
issue adequately. However, I'd like to hear a second opinion on the
possible side effects of this fix, since NET_Timeout may be called
at many other places.

I see that Alan has commented on
https://bugs.openjdk.java.net/browse/JDK-8237858

It would be good to get his approval (or at least Michael McMahon's) before pushing.

best regards,

-- daniel

On 25/02/2020 16:36, Vyom Tewari26 wrote:
Hi ,
Please find the below fix for the issue(https://bugs.openjdk.java.net/browse/JDK-8237858). In "PlainSocketImpl_socketAccept" did not handle -1 timeout properly. In case of -1 timeout, "PlainSocketImpl_socketAccept" calls "NET_Timeout"  if it is interrupted by signal(EINTR) then in case of -1 timeout it returns immediately instead of looping again.
Thanks,
Vyom
##########################################################
diff -r d6b968af8b65 src/java.base/linux/native/libnet/linux_close.c
--- a/src/java.base/linux/native/libnet/linux_close.c    Mon Feb 24 23:44:29 2020 -0500 +++ b/src/java.base/linux/native/libnet/linux_close.c    Tue Feb 25 19:06:11 2020 +0530
@@ -437,12 +437,16 @@
           * has expired return 0 (indicating timeout expired).
           */
          if (rv < 0 && errno == EINTR) {
-            jlong newNanoTime = JVM_NanoTime(env, 0);
-            nanoTimeout -= newNanoTime - prevNanoTime;
-            if (nanoTimeout < NET_NSEC_PER_MSEC) {
-                return 0;
+            if(timeout > 0) {
+                jlong newNanoTime = JVM_NanoTime(env, 0);
+                nanoTimeout -= newNanoTime - prevNanoTime;
+                if (nanoTimeout < NET_NSEC_PER_MSEC) {
+                    return 0;
+                }
+                prevNanoTime = newNanoTime;
+            } else {
+                continue; // timeout is -1, so  loop again.
              }
-            prevNanoTime = newNanoTime;
          } else {
              return rv;
          }

############################################################


Reply via email to