+1
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Mark
Sheppard
Sent: Donnerstag, 1. September 2016 19:51
To: Vyom Tewari <vyom.tew...@oracle.com>; net-dev@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
