Re: [DONG] Re: [DING] Re: [PING] Potential infinite waiting at JMXConnection#createConnection

2016-02-08 Thread David Holmes 

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 :

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/classes/sun/rmi/transport/tcp/TCPChannel.java#l227

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/file/e31044f0804f/jdk9.patch

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 :

Hi all,

Can somebody please review and sponsor this fix ?

Thanks,
Yuji

2016-01-05 17:56 GMT+09:00 KUBOTA Yuji :

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/file/e31044f0804f/jdk9.patch

  * testcase for reproduce
  
http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/file/e31044f0804f/testProgram
  
http://mail.openjdk.java.net/pipermail/serviceability-dev/2015-December/018415.html

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

Thanks,
Yuji

2016-01-04 23:51 GMT+09:00 Jaroslav Bachorik :

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/file/e31044f0804f/jdk9.patch)

Regards,

-JB-

Re: [DONG] Re: [DING] Re: [PING] Potential infinite waiting at JMXConnection#createConnection

2016-02-08 Thread KUBOTA Yuji 
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 :
> 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 :
>>>
>>> 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/classes/sun/rmi/transport/tcp/TCPChannel.java#l227
>>>
>>> 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, thi