[DONG] Re: [DING] Re: [PING] Potential infinite waiting at JMXConnection#createConnection
KUBOTA Yuji
kubota.yuji at gmail.com
Tue Feb 9 06:50:15 UTC 2016
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 at 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 at 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/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 <kubota.yuji at 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 at 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/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
>>>>> <jaroslav.bachorik at 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/file/e31044f0804f/jdk9.patch)
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> -JB-
More information about the core-libs-dev
mailing list