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

KUBOTA Yuji kubota.yuji at gmail.com
Thu Mar 3 15:03:01 UTC 2016


Hi all,

Could someone please review this patch?

Thanks,
Yuji

2016-02-09 15:50 GMT+09:00 KUBOTA Yuji <kubota.yuji at 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 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