Hi all,
Could someone please review this patch?
Thanks,
Yuji
2016-02-09 15:50 GMT+09:00 KUBOTA Yuji <kubota.y...@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.hol...@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.y...@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.y...@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.y...@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.bacho...@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-
