Hi all, Could someone please review this patch? Thanks, Yuji 2016-02-09 15:50 GMT+09:00 KUBOTA Yuji <kubota.yuji@gmail.com>:

Hi David,

Thank you for your advice and cc-ing!

I do not have any role yet, so I paste my patches as below.

diff --git a/src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java b/src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java --- a/src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java +++ b/src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java @@ -222,20 +222,34 @@ // choose protocol (single op if not reusable socket) if (!conn.isReusable()) { out.writeByte(TransportConstants.SingleOpProtocol); } else { out.writeByte(TransportConstants.StreamProtocol); + + int usableSoTimeout = 0; + try { + /* + * If socket factory had set a non-zero timeout on its + * own, then restore it instead of using the property- + * configured value. + */ + usableSoTimeout = sock.getSoTimeout(); + if (usableSoTimeout == 0) { + usableSoTimeout = responseTimeout; + } + sock.setSoTimeout(usableSoTimeout); + } catch (Exception e) { + // if we fail to set this, ignore and proceed anyway + } out.flush();

/* * Set socket read timeout to configured value for JRMP * connection handshake; this also serves to guard against * non-JRMP servers that do not respond (see 4322806). */ - int originalSoTimeout = 0; try { - originalSoTimeout = sock.getSoTimeout(); sock.setSoTimeout(handshakeTimeout); } catch (Exception e) { // if we fail to set this, ignore and proceed anyway }

@@ -279,18 +293,11 @@ * connection. NOTE: this timeout, if configured to a * finite duration, places an upper bound on the time * that a remote method call is permitted to execute. */ try { - /* - * If socket factory had set a non-zero timeout on its - * own, then restore it instead of using the property- - * configured value. - */ - sock.setSoTimeout((originalSoTimeout != 0 ? - originalSoTimeout : - responseTimeout)); + sock.setSoTimeout(usableSoTimeout); } catch (Exception e) { // if we fail to set this, ignore and proceed anyway }

out.flush();

Thanks, Yuji

2016-02-09 13:11 GMT+09:00 David Holmes <david.holmes@oracle.com>:

...

Hi Yuji,

Not sure who would look at this so cc'ing net-dev.

Also note that contributions can only be accepted if presented via OpenJKDK infrastructure. Links to patches on http://icedtea.classpath.org are not acceptable. The patch needs to be included in the email (beware stripped attachments) if you can't get it hosted on cr.openjdk.java.net. Sorry.

David

On 9/02/2016 12:10 AM, KUBOTA Yuji wrote:

...

Hi all,

Could someone review this fix?

Thanks, Yuji

2016-02-04 2:27 GMT+09:00 KUBOTA Yuji <kubota.yuji@gmail.com>:

...

Hi all,

Could someone please review and sponsor this fix ? I write the details of this issue again. Please review it.

=Problem= Potential infinite waiting at TCPChannel#createConnection.

This method flushes the DataOutputStream without the socket timeout settings when choose stream protocol [1]. If connection lost or the destination server do not return response during the flush, this method wait forever because the timeout settings is set the default value of SO_TIMEOUT, i.e., infinite.

[1]: http://hg.openjdk.java.net/jdk9/dev/jdk/file/7adef1c3afd5/src/java.rmi/share...

I think this issue is rarely, however serious.

=Reproduce= I write a test program to reproduce. You can reproduce by the below.

* hg clone http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/ * cd fixLoopAtJMXConnectorFactory; mvn package * setting "stop_time" at debugcontrol.properties if you need. * java -cp .:target/debugcontrol-1.0-SNAPSHOT.jar debugcontrol.DebugController

This program keep to wait at TCPChannel#createConnection due to this issue. After "debugcontroltest.stop_time" ms, this program release the waiting by sending quit to jdb which is stopping the destination server. Finally, return 2.

=Solution= Set timeout by using property-configured value: sun.rmi.transport.tcp.responseTimeout.

My patch is below.

http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/fil...

If you run the test program with modified JDK9 by my patch, the test program will get java.net.SocketTimeoutException after the connection timeout happen, then return 0.

Thanks, Yuji.

2016-01-13 23:31 GMT+09:00 KUBOTA Yuji <kubota.yuji@gmail.com>:

...

Hi all,

Can somebody please review and sponsor this fix ?

Thanks, Yuji

2016-01-05 17:56 GMT+09:00 KUBOTA Yuji <kubota.yuji@gmail.com>:

...

Hi Jaroslav and core-libs-dev,

Thank Jaroslav for your kindness!

For core-libs-dev members, links the information about this issue.

* details of problem

http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-April/002152.html

* patch

http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/fil...

* testcase for reproduce

http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/fil...

http://mail.openjdk.java.net/pipermail/serviceability-dev/2015-December/0184...

Could you please review these reports? Hope this patch helps to community.

Thanks, Yuji

2016-01-04 23:51 GMT+09:00 Jaroslav Bachorik <jaroslav.bachorik@oracle.com>: > > Hi Yuji, > > On 4.1.2016 15:14, KUBOTA Yuji wrote: >> >> >> Hi all, >> >> Could you please review this patch? > > > > Sorry for the long delay. Shanliang has not been present for some time > and > probably this slipped the attention of the others. > > However, core-libs mailing list might be more appropriate place to > review > this change since you are dealing with s.r.t.t.TCPChannel > > (http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/fil...) > > Regards, > > -JB-