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

[Bernd Eckenfels]

Am Thu, 1 Sep 2016 23:08:15 +0300
schrieb Dmitry Samersoff <dmitry.samersoff at oracle.com>:

> gettimeofday is affected by system time changes, so running ntpd can
> cause unpredictable behavior of this code. Also it's rather expensive
> syscall.

At least on Linux It is actually not a syscall at all and very
performant (on Linux gettimeofday and clock_gettime are VDSOs).

But I would agree, for timeouts a clock_gettime(CLOCK_MONOTONIC_COARSE
or RAW) should be used.

However, this really should be a general JVM abstraction...

Gruss
Bernd

> 
> -Dmitry
> 
> On 2016-09-01 19: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>
> >>>>
> >>>> 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
> >>>>
> >>>>
> >>>
> >>
> > 
> 
>