RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

[Langer, Christoph]

+1

From: net-dev [mailto:net-dev-bounces at openjdk.java.net] On Behalf Of Mark Sheppard
Sent: Donnerstag, 1. September 2016 19:51
To: Vyom Tewari <vyom.tewari at oracle.com>; net-dev at openjdk.java.net
Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with soTimeout set

Hi Vyom,
     thanks for doing the refactoring.
changes look OK to me

regards
Mark
On 01/09/2016 17:03, Vyom Tewari wrote:

please find the updated webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.1/index.html>). I incorporated the review comments.

Thanks,

Vyom

On Tuesday 30 August 2016 04:11 PM, Mark Sheppard wrote:

Hi
  perhaps there is an opportunity to do  some refactoring here (...  for me a "goto " carries a code smell! )

along the lines

if (timeout) {
    nread =  NET_ReadWithTimeout(...);
} else {
     nread = NET_Read(...);
}


the NET_ReadWithTimeout (...) function will contain a restructuring of your goto loop

        while (_timeout > 0) {

          nread = NET_Timeout(fd, _timeout);

          if (nread <= 0) {

              if (nread == 0) {

                  JNU_ThrowByName(env, JNU_JAVANETPKG "SocketTimeoutException",

                              "Read timed out");

              } else if (nread == -1) {

                  if (errno == EBADF) {

                       JNU_ThrowByName(env, JNU_JAVANETPKG "SocketException", "Socket closed");

                  } else if (errno == ENOMEM) {

                      JNU_ThrowOutOfMemoryError(env, "NET_Timeout native heap allocation failed");

                  } else {

                      JNU_ThrowByNameWithMessageAndLastError

                          (env, JNU_JAVANETPKG "SocketException", "select/poll failed");

                  }

              }

                // release buffer in main call flow

//              if (bufP != BUF) {

//                  free(bufP);

//             }

             nread = -1;

             break;

          } else {

              nread = NET_NonBlockingRead(fd, bufP, len);

              if (nread == -1 && ((errno == EAGAIN) || (errno == EWOULDBLOCK))) {

                  gettimeofday(&t, NULL);

                  newtime = t.tv_sec * 1000 + t.tv_usec / 1000;

                  _timeout -= newtime - prevtime;

                  if(_timeout > 0){

                      prevtime = newtime;

                  }

              } else {

                  break;

              }

          }

       }



        return nread;



e&oe

regards
Mark

On 29/08/2016 10:58, Vyom Tewari wrote:
gentle reminder, please review the below code change.

Vyom


On Monday 22 August 2016 05:12 PM, Vyom Tewari wrote:

Hi All,

Please review the code changes for below issue.

Bug         : https://bugs.openjdk.java.net/browse/JDK-8075484

webrev    : http://cr.openjdk.java.net/~vtewari/8075484/webrev0.0/index.html<http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.0/index.html> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.0/index.html><http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.0/index.html>

This issue is SocketInputStream.socketread0() hangs even with "soTimeout" set.the implementation of Java_java_net_SocketInputStream_socketRead0 assumes that read() won't block after poll() reports that a read is possible.

This assumption does not hold, as noted on the man page for select (referenced by the man page for poll): Under Linux, select() may report a socket file descriptor as "ready for reading", while nevertheless a subsequent read blocks. This could for example happen when data has arrived but upon examination has wrong checksum and is discarded.

Thanks,

Vyom





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20160901/b13dce90/attachment.html>