On 10 August 2021 16:41:00 CEST, Sylvain POGNANT <[email protected]> wrote: >Hi, > >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; > > >
Please raise a JIRA with steps / code to reproduce or better yet raise a PR at GitHub with a test case and a proposed fix. Oleg -- Sent from my Android device with K-9 Mail. Please excuse my brevity. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
