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

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Langer, Christoph

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

 

Reply | Threaded
Open this post in threaded view
|

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

Vyom Tewari

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

 


Reply | Threaded
Open this post in threaded view
|

RE: [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:[hidden email]]
Sent: Donnerstag, 29. Dezember 2016 11:09
To: Langer, Christoph <[hidden email]>; [hidden email]; [hidden email]
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