... and one additional minor remark from my side: Please also check if the #include <sys/time.h> can be left out from src/java.base/unix/native/libnet/SocketInputStream.c - I don't think it's necessary now as the new "NET_ReadWithTimeout" function does not directly access time APIs.
Other than that I'm fine with the change but I still want to test MSG_NONBLOCK on AIX. So far I've only verified that it builds but MSG_NONBLOCK is not officially documented for AIX. I've only heard from some experts that it "should work" but I'd like to see it in operation. But don't hold back your stuff for that, if worst comes to worse I'll have to do an AIX specific fix. Best regards Christoph > -----Original Message----- > From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Chris > Hegarty > Sent: Dienstag, 13. September 2016 10:48 > To: Vyom Tewari <vyom.tew...@oracle.com> > Cc: net-dev@openjdk.java.net > Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even with > soTimeout set > > Vyom, > > > On 13 Sep 2016, at 04:00, Vyom Tewari <vyom.tew...@oracle.com> wrote: > > > > any comment on latest > webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html) ? > > I am happy with the way this change turned out. Reviewed. > > Trivially, before you push, just add a space to the four OS specific versions > of > Net_Timeout0, as follows: > > NET_Timeout0(int s, long timeout, long currentTime) > ^^^ > Thanks, > -Chris. > > > Vyom > > > > On Thursday 08 September 2016 03:13 PM, Vyom Tewari wrote: > >> > >> On Thursday 08 September 2016 02:50 PM, Langer, Christoph wrote: > >>> Hi Vyom, > >>> > >>> this looks good. > >>> > >>> Is there any particular reason why NET_ReadWithTimeout should remain in > SocketInputStream.c and not also move to net_util_md.c? That way you could > also have a "static long getCurrentTime()" inside net_util_md.c, instead of an > exported NET_GetCurrentTime(). > >>> > >> I thought this "NET_ReadWithTimeou" is specific to SocketInputStream so i > will be good if we put this method to socketinputstream.c. > >> > >> In future we will in using the monotonic increasing clock so > "NET_GetCurrentTime()" will help , Just change the implementation you are > done. > >> > >> Vyom > >>> > >>> And no "#include <sys/time.h>" would be needed in SocketInputStream.c - > maybe not even now as it is. > >>> > >>> Best regards > >>> Christoph > >>> > >>> > >>>> -----Original Message----- > >>>> From: Vyom Tewari [ > >>>> mailto:vyom.tew...@oracle.com > >>>> ] > >>>> Sent: Donnerstag, 8. September 2016 05:20 > >>>> To: Langer, Christoph > >>>> <christoph.lan...@sap.com> > >>>> > >>>> Cc: > >>>> net-dev@openjdk.java.net > >>>> > >>>> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang even > with > >>>> soTimeout set > >>>> > >>>> Hi All, > >>>> > >>>> Please find the latest > >>>> webrev( > >>>> http://cr.openjdk.java.net/~vtewari/8075484/webrev0.5/index.html > >>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.5/index.html> > >>>> ). > >>>> > >>>> I fixed the AIX flag issue and move some come common code to > >>>> "net_util_md.c" file. > >>>> > >>>> Thanks, > >>>> Vyom > >>>> > >>>> On 9/8/2016 12:32 PM, Langer, Christoph wrote: > >>>> > >>>>> Hi Vyom, > >>>>> > >>>>> ok, thanks. I have one addition to my proposal: I don't think there's a > need for > >>>>> > >>>> a global NET_GetCurrentTime function. This one should probably be a > static > >>>> function inside net_util_md.c (static long getCurrentTime()). > >>>> > >>>>> Best > >>>>> Christoph > >>>>> > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Vyom Tewari [ > >>>>>> mailto:vyom.tew...@oracle.com > >>>>>> ] > >>>>>> Sent: Mittwoch, 7. September 2016 18:31 > >>>>>> To: Langer, Christoph > >>>>>> <christoph.lan...@sap.com> > >>>>>> > >>>>>> Cc: > >>>>>> net-dev@openjdk.java.net; Chris Hegarty <chris.hega...@oracle.com> > >>>>>> > >>>>>> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang > even > >>>>>> > >>>> with > >>>> > >>>>>> soTimeout set > >>>>>> > >>>>>> Hi Chris, > >>>>>> > >>>>>> Sorry I was mostly focusing on our test failure first, i will check > >>>>>> your suggestions. > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> Vyom > >>>>>> > >>>>>> > >>>>>> On Wednesday 07 September 2016 08:44 PM, Langer, Christoph wrote: > >>>>>> > >>>>>>> Hi Vyom, > >>>>>>> > >>>>>>> did you have a look at my suggestions regarding AIX and refactoring? I > >>>>>>> > >>>> can't > >>>> > >>>>>> find none of it in the new webrev nor did you comment on it. > >>>>>> > >>>>>>> Best regards > >>>>>>> Christoph > >>>>>>> > >>>>>>> > >>>>>>>> -----Original Message----- > >>>>>>>> From: net-dev [ > >>>>>>>> mailto:net-dev-boun...@openjdk.java.net > >>>>>>>> ] On Behalf Of > >>>>>>>> > >>>>>> Vyom > >>>>>> > >>>>>>>> Tewari > >>>>>>>> Sent: Mittwoch, 7. September 2016 17:10 > >>>>>>>> To: Chris Hegarty > >>>>>>>> <chris.hega...@oracle.com> > >>>>>>>> > >>>>>>>> Cc: > >>>>>>>> net-dev@openjdk.java.net > >>>>>>>> > >>>>>>>> Subject: Re: RFR 8075484:SocketInputStream.socketRead0 can hang > even > >>>>>>>> > >>>>>> with > >>>>>> > >>>>>>>> soTimeout set > >>>>>>>> > >>>>>>>> Hi All, > >>>>>>>> > >>>>>>>> Please find the latest > >>>>>>>> > >>>>>>>> > >>>> > webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.4/index.html > >>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.4/index.html> > >>>> ). > >>>> > >>>>>>>> there were some test failures in JPRT and Chris also pointed the > same > >>>>>>>> issue in modified code. Now with latest code JPRT is clean. > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> > >>>>>>>> Vyom > >>>>>>>> > >>>>>>>> > >>>>>>>> On Wednesday 07 September 2016 03:18 PM, Chris Hegarty wrote: > >>>>>>>> > >>>>>>>>> On 07/09/16 07:45, Vyom Tewari wrote: > >>>>>>>>> > >>>>>>>>>> Hi Chris, > >>>>>>>>>> > >>>>>>>>>> Please find the latest > >>>>>>>>>> > >>>>>>>>>> > >>>>>> > webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.3/index.html > >>>>>> > <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.3/index.html> > >>>>>> ). I > >>>>>> > >>>>>>>>>> did some code refactoring, JPRT is in progress. > >>>>>>>>>> > >>>>>>>>> In terms of NET_Timeout**, I'm happy with this version, but I > >>>>>>>>> have an additional comment. > >>>>>>>>> > >>>>>>>>> If NET_ReadWithTimeout returns -1 because > >>>>>>>>> > >>>>>> NET_TimeoutWithCurrentTime > >>>>>> > >>>>>>>>> returns <= 0, then a JNI pending exception will be set. So the code > >>>>>>>>> calling NET_ReadWithTimeout should, a) free bufP, and b) return - > 1, > >>>>>>>>> immediately rather than continuing and possibly trying to set > another > >>>>>>>>> JNI pending exception. > >>>>>>>>> > >>>>>>>>> One option is to check the return value from > NET_ReadWithTimeout and > >>>>>>>>> also (*env)->ExceptionCheck(env). Another is to possibly consolidate > >>>>>>>>> the setting of JNI pending exceptions in one place, towards the end > >>>>>>>>> of Java_java_net_SocketInputStream_socketRead0, for example > EBADF > >>>>>>>>> > >>>> is > >>>> > >>>>>>>>> handled in two places. > >>>>>>>>> > >>>>>>>>> -Chris. > >>>>>>>>> > >>>>>>>>> > >>>>>>>>>> I need help from some one to build and test latest changes on AIX, > may > >>>>>>>>>> be Christoph can help. > >>>>>>>>>> > >>>>>>>>>> Thanks, > >>>>>>>>>> > >>>>>>>>>> Vyom > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Tuesday 06 September 2016 06:25 PM, Chris Hegarty wrote: > >>>>>>>>>> > >>>>>>>>>>> Vyom, > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>>> On 6 Sep 2016, at 10:20, Vyom Tewari > <vyom.tew...@oracle.com> > >>>>>> wrote: > >>>>>> > >>>>>>>>>>>> Hi, > >>>>>>>>>>>> > >>>>>>>>>>>> Please find the latest > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>> > webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.2/index.html > >>>> <http://cr.openjdk.java.net/%7Evtewari/8075484/webrev0.2/index.html> > >>>> ). > >>>> > >>>>>>>>>>>> I > >>>>>>>>>>>> have incorporated the review comments. > >>>>>>>>>>>> > >>>>>>>>>>> Your changes completely avoid NET_Timeout, which is quite > complex > >>>>>>>>>>> > >>>> on > >>>> > >>>>>>>>>>> Linux, Mac, and AIX, for various reasons. ( NET_Timeout will use > the > >>>>>>>>>>> async close mechanism to signal/interrupt a thread blocked in a > poll / > >>>>>>>>>>> select ). Also, select is used on Mac, which will use poll after > >>>>>>>>>>> your > >>>>>>>>>>> changes ( I do remember that there was a bug in poll on Mac > around > >>>>>>>>>>> the time of the original Mac port ). > >>>>>>>>>>> > >>>>>>>>>>> Would is be better, rather than replicating the logic in > NET_Timeout, > >>>>>>>>>>> to have a version that supports passing a function pointer, to run > if > >>>>>>>>>>> the poll/select returns before the timeout? Just an idea. > >>>>>>>>>>> > >>>>>>>>>>> -Chris. > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>>> Thanks, > >>>>>>>>>>>> > >>>>>>>>>>>> Vyom > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> On Monday 05 September 2016 08:37 PM, Chris Hegarty wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> On 05/09/16 15:37, Mark Sheppard wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> if the desire is to avoid making double calls on gettimeofday > in the > >>>>>>>>>>>>>> NET_ReadWithTimeout's while loop for its main call flow, > >>>>>>>>>>>>>> > >>>>>>>>>>>>> Yes, the desire is to make no more calls to gettimeofday, than > are > >>>>>>>>>>>>> already done. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> then consider a modification to NET_Timeout and create a > version > >>>>>>>>>>>>>> int NET_TimeoutWithCurrentTime(int s, long timeout, stuct > >>>>>>>>>>>>>> > >>>> timeval * > >>>> > >>>>>>>>>>>>>> current_time) > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> int NET_TimeoutWithCurrentTime(int s, long timeout, stuct > >>>>>>>>>>>>>> > >>>> timeval * > >>>> > >>>>>>>>>>>>>> current_time) { > >>>>>>>>>>>>>> int result; > >>>>>>>>>>>>>> struct timeval t; > >>>>>>>>>>>>>> long prevtime, newtime; > >>>>>>>>>>>>>> struct pollfd pfd; > >>>>>>>>>>>>>> pfd.fd = s; > >>>>>>>>>>>>>> pfd.events = POLLIN; > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> if (timeout > 0) { > >>>>>>>>>>>>>> prevtime = (current_time->tv_sec * 1000) + > >>>>>>>>>>>>>> current_time->tv_usec / 1000; > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> for(;;) { > >>>>>>>>>>>>>> result = poll(&pfd, 1, timeout); > >>>>>>>>>>>>>> if (result < 0 && errno == EINTR) { > >>>>>>>>>>>>>> if (timeout > 0) { > >>>>>>>>>>>>>> gettimeofday(&t, NULL); > >>>>>>>>>>>>>> newtime = (t.tv_sec * 1000) + t.tv_usec > >>>>>>>>>>>>>> /1000; > >>>>>>>>>>>>>> timeout -= newtime - prevtime; > >>>>>>>>>>>>>> if (timeout <= 0) > >>>>>>>>>>>>>> return 0; > >>>>>>>>>>>>>> prevtime = newtime; > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> } else { > >>>>>>>>>>>>>> return result; > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> } > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> the NET_ReadWithTimeout is modified with. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> while (timeout > 0) { > >>>>>>>>>>>>>> result = NET_TimeoutWithCurrentTime(fd, timeout, > &t); > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> in any case there is the potential for multiple invocation of > >>>>>>>>>>>>>> gettimeofday due to a need to make > >>>>>>>>>>>>>> poll restartable, > >>>>>>>>>>>>>> > >>>>>>>>>>>>> Yes, and that is fine. Just no more than is already there. > >>>>>>>>>>>>> > >>>>>>>>>>>>> and adjust the originally specified timeout > >>>>>>>>>>>>> > >>>>>>>>>>>>>> accordingly, and for the edge case > >>>>>>>>>>>>>> after the non blocking read. > >>>>>>>>>>>>>> > >>>>>>>>>>>>> After an initial skim, your suggestion seems reasonable. > >>>>>>>>>>>>> > >>>>>>>>>>>>> -Chris. > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>>> regards > >>>>>>>>>>>>>> Mark > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On 05/09/2016 12:02, Chris Hegarty wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Vyom, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>> > webrev(http://cr.openjdk.java.net/~vtewari/8075484/webrev0.1/index.html > >>>>>>>>>>>>>>> There is some concern about the potential performance > effect, > >>>>>>>>>>>>>>> > >>>> etc, > >>>> > >>>>>>>>>>>>>>> of gettimeofday, maybe there is a way out of this. The reuse > of > >>>>>>>>>>>>>>> NET_Timeout is good, but it also calls gettimeofday. It > seems that > >>>>>>>>>>>>>>> a specific NET_ReadWithTimeout could be written to NOT > reuse > >>>>>>>>>>>>>>> NET_Timeout, but effectively inline its interruptible > operation. > >>>>>>>>>>>>>>> Or write a variant of NET_Timeout that takes a function to > >>>>>>>>>>>>>>> execute. Rather than effectively two loops conditioned on > the > >>>>>>>>>>>>>>> timeout. Disclaimer: I have not actually tried to do this, > >>>>>>>>>>>>>>> but > >>>>>>>>>>>>>>> it seems worth considering / evaluating. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> -Chris. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On 02/09/16 04:39, Vyom Tewari wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> hi Dimitry, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> thanks for review, I did consider to use a monotonically > >>>>>>>>>>>>>>>> increasing > >>>>>>>>>>>>>>>> clock like "clock_gettime" but existing nearby > >>>>>>>>>>>>>>>> code("NET_Timeout") uses > >>>>>>>>>>>>>>>> "gettimeofday" so i choose to be consistent with the > existing > >>>>>>>>>>>>>>>> code. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Vyom > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Friday 02 September 2016 01:38 AM, Dmitry Samersoff > >>>>>>>>>>>>>>>> > >>>> wrote: > >>>> > >>>>>>>>>>>>>>>>> Vyom, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Did you consider to use select() to calculate timeout > instead of > >>>>>>>>>>>>>>>>> gettimeofday ? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> gettimeofday is affected by system time changes, so > running > >>>>>>>>>>>>>>>>> > >>>> ntpd > >>>> > >>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>> cause unpredictable behavior of this code. Also it's rather > >>>>>>>>>>>>>>>>> expensive > >>>>>>>>>>>>>>>>> syscall. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> -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 > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >> > >
