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 >>>> >>>> >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
