Hi Felix,
 
Thanks for review, writing a test case is not straight forward. 
 
we have to manually inject the write failure, if you see the attached reproducer with the issue it  hacks the send native API and inject  on the client side which simulates a transient network failure.
 
Even i was not able to reproduce the issue with hooks provided by submitter with latest JDK code.  I was not able inject the failure so HttpURLConnect.writeRequests() did not follow the path where we retry and create new HttpClient, but with JDK8 i was able to reproduce the issue and after my fix issue got resolved.
 
 
Thanks,
Vyom
----- Original message -----
From: "felixxfyang(杨晓峰)" <felixxfy...@tencent.com>
Sent by: "core-libs-dev" <core-libs-dev-boun...@openjdk.java.net>
To: Vyom Tiwari <vyomm...@gmail.com>, net-dev <net-dev@openjdk.java.net>, core-libs-dev <core-libs-...@openjdk.java.net>
Cc:
Subject: [EXTERNAL] Re: RFR 8238579: HttpsURLConnection drops the timeout and hangs forever in read(Internet mail)
Date: Fri, Feb 14, 2020 12:01 PM
 
Hi Vyom,
  The patch looks fine. Can you add a regression test for it?

Thanks,
Felix Yang
在 2020/2/14 下午1:01,“core-libs-dev 代表 Vyom Tiwari”<core-libs-dev-boun...@openjdk.java.net 代表 vyomm...@gmail.com> 写入:

    Hi All,
    
    Please find the below fix  which resolves the issue(
    https://bugs.openjdk.java.net/browse/JDK-8238579 ).
    
    "HttpURLConnection.writeRequests()" retry in case of any write failure,
    during retry it creates new HttpsClient  without connectTimeout &
    readTimeout. Below fix sets the connect & read timeout.
    
    Thanks,
    Vyom
    
    diff -r 7e6165c9c606
    src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
    ---
    a/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
    Thu Feb 13 17:14:45 2020 -0800
    +++
    b/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
    Fri Feb 14 10:11:06 2020 +0530
    @@ -87,10 +87,15 @@
          */
         public void setNewClient (URL url, boolean useCache)
             throws IOException {
    +        int readTimeout = getReadTimeout();
             http = HttpsClient.New (getSSLSocketFactory(),
                                     url,
                                     getHostnameVerifier(),
    -                                useCache, this);
    + null,
    + -1,
    +                                useCache,
    + getConnectTimeout(), this);
    + http.setReadTimeout(readTimeout);
             ((HttpsClient)http).afterConnect();
         }
    
    @@ -132,10 +137,14 @@
                 boolean useCache) throws IOException {
             if (connected)
                 return;
    -        http = HttpsClient.New (getSSLSocketFactory(),
    -                                url,
    -                                getHostnameVerifier(),
    -                                proxyHost, proxyPort, useCache, this);
    +        int readTimeout = getReadTimeout();
    +        http = HttpsClient.New(getSSLSocketFactory(),
    +                url,
    +                getHostnameVerifier(),
    +                proxyHost, proxyPort,
    +                useCache,
    +                getConnectTimeout(), this);
    +        http.setReadTimeout(readTimeout);
             connected = true;
         }
    
    

 
 

Reply via email to