You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Sylvain POGNANT <s....@overkiz.com> on 2021/08/10 14:41:00 UTC
H2ConnPool bug (stuck poolEntry)
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;
--
Overkiz Logo <http://www.overkiz.com> *Sylvain POGNANT*
Lead Architect
s.pognant@overkiz.com <ma...@overkiz.com>
Immeuble Variation A | Allée de la Mandallaz
74370 METZ-TESSY | France
www.overkiz.com <http://www.overkiz.com>
Overkiz Footer <http://www.overkiz.com>
This message and any attachments are confidential and intended solely
for the addressees. Any unauthorized use or disclosure, either whole or
partial is prohibited. E-mails are susceptible to alteration. OVERKIZ
shall not be liable for the message if altered, changed or falsified. If
you are not the intended recipient of this message, please delete it and
notify the sender.
Please think of the environment before printing this email.
Re: H2ConnPool bug (stuck poolEntry)
Posted by Oleg Kalnichevski <ol...@ok2consulting.com>.
On 10 August 2021 16:41:00 CEST, Sylvain POGNANT <s....@overkiz.com> 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org