[HTTPCORE-683] H2ConnPool bug (stuck poolEntry)

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 5.1.1
• Fix Version/s: 5.1.2, 5.2-alpha1
• Component/s: HttpCore
• Environment: Linux / Ubuntu / openjdk version "1.8.0_292"
• Assignee: Unassigned
• Reporter: Sylvain POGNANT
• Created: 13/Aug/21 07:28
• Updated: 01/Jun/23 23:22
• Resolved: 01/Sep/21 20:42

Description

There is an implementation problem in AbstractIOSessionPool.getSessionInternal()

Wrong pattern (starting line 190 in httpcore5-5.1.1) :

if (poolEntry.sessionFuture == null) {
                    poolEntry.sessionFuture = connectSession(
                            namedEndpoint,
                            connectTimeout,
                            new FutureCallback<IOSession>() {

                               ...

                                @Override
                                public void failed(final Exception ex) {
                                    synchronized (poolEntry)

{                                         poolEntry.session = null;                                         *poolEntry.sessionFuture = null;*                                         ...                                     }

                                }
                             ...

                            });

In case of a synchronous failure during connectSession (for example : UnknownHostException), the sessionFuture cleanup in the failed() callback (line 216) is executed before poolEntry.sessionFuture is assigned with the return value of connectSession (line 191)

Thus, after the failure, a cancelled future is assigned to poolEntry.sessionFuture and never cleaned up !

This result in a stalled entry in the pool because the if (poolEntry.sessionFuture == null) condition will no longer return true.

 

Quick Reproducer

       H2AsyncClientBuilder httpClientBuilder = HttpAsyncClients.customHttp2()
                        .setDefaultRequestConfig(RequestConfig.custom()
                                        .setConnectTimeout(5000, TimeUnit.MILLISECONDS)
                                        .setResponseTimeout(5000, TimeUnit.MILLISECONDS)
                                        .build())
                        .setH2Config(H2Config.custom()
                                        .setPushEnabled(false)
                                        .build());
               
        CloseableHttpAsyncClient client = httpClientBuilder.build();
        client.start();
       
        for(int n=1;n<=2;n++)
        {
            System.out.println("Try #"+n);   
           
            HttpHost targetHost = new HttpHost(URIScheme.HTTPS.getId(), "invalidhost-gfhsgfxyzd.com", 443);
            SimpleHttpRequest request = new SimpleHttpRequest("GET", targetHost, "/");
   
            Future<SimpleHttpResponse> responseFuture = client.execute(request, null);
            try
            {
                responseFuture.get();  // HANGS FOREVER at Try #2
            }
            catch (ExecutionException e)
           

{                 e.printStackTrace();             }

        }

 

Quick Fix

Cleaning the sessionFuture immediately after connectSession() returns in case its already done fixes the issue :

(Line 234)

+ if (poolEntry.sessionFuture != null && poolEntry.sessionFuture.isDone())
+                       poolEntry.sessionFuture = null;

Issue Links

• is cloned by: HTTPCORE-750 H2ConnPool creates too many conns under high load at initialization time