[8u-dev]: Request for Review and Approval: 8075484: SocketInputStream.socketRead0 can hang even with soTimeout set

[Langer, Christoph]

Hi Vyom,

generally, I think it is a good idea to have a look at NET_GetCurrentTime vs. JVM_CurrentTimeMillis again.

However, NET_GetCurrentTime is self-contained and does not need arguments whereas JVM_CurrentTimeMillis needs a JNI env pointer. And since NET_GetCurrentTime is called from NET_Timeout which currently does not have access to the JNI env pointer, it is not so easy to change this as I'd have to touch all places where NET_Timeout is called. So, for this downport I think we should leave this as is.

But, as for JDK9 (or maybe JDK10) I think a change should be done to either go for JVM_CurrentTimeMillis which means touching all NET_Timeout calls or, alternatively, change all JVM_CurrentTimeMillis to NET_GetCurrentTime in libnet. Maybe the latter one is even more desirable as the local call could be a bit cheaper than calling into hotspot. I filed bug 8172125 for that and assigned it to me.

Best regards
Christoph



From: Vyom Tewari [mailto:vyom.tewari at oracle.com]
Sent: Donnerstag, 29. Dezember 2016 11:09
To: Langer, Christoph <christoph.langer at sap.com>; net-dev at openjdk.java.net; jdk8u-dev at openjdk.java.net
Subject: Re: [8u-dev]: Request for Review and Approval: 8075484: SocketInputStream.socketRead0 can hang even with soTimeout set


Hi Christoph,

As this is not direct backport what about using the existing function "JVM_CurrentTimeMillis(env, 0);" instead of " NET_GetCurrentTime() ". When i did this code change i did not know that we already have this.If you use the existing one then you can simply remove the " NET_GetCurrentTime() "  from your code change.

Thanks,

Vyom

On Thursday 29 December 2016 03:07 PM, Langer, Christoph wrote:
Hi,

please review (and eventually approve) the change for downporting 8075484.

Webrev for 8u-dev: http://cr.openjdk.java.net/~clanger/webrevs/8075484.8udev/
Bug: https://bugs.openjdk.java.net/browse/JDK-8075484
JDK9 Change: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/af17b6bc08dd
JDK9 Review Thread(s):
  http://mail.openjdk.java.net/pipermail/net-dev/2016-August/010171.html
   http://mail.openjdk.java.net/pipermail/net-dev/2016-September/010201.html

We had customer reports who ran into that issue with Java 8. So this should be downported.

The problem is, that the fix does not apply to Solaris as Solaris needs some calls into hotspot. This is because in JDK 8 the flag for interruptible IO is still supported (though deprecated). But I think it is still worthwile to bring this down for the other platforms which I'm proposing with my changeset. So I extracted the new code manually from the JDK9 changeset and made it fit into JDK8 coding.

Thanks & Best regards
Christoph