Issue #2860 Fix leakage of HttpDestinations in HttpClient

Signed-off-by: Ivanov Anton <an.ivanov@corp.mail.ru>
This commit is contained in:
Ivanov Anton 2018-08-25 18:27:18 +03:00
parent bd143a674b
commit 084d6ce443
2 changed files with 48 additions and 10 deletions

View File

@ -210,16 +210,7 @@ public abstract class PoolingHttpDestination<C extends Connection> extends HttpD
if (getHttpExchanges().isEmpty())
{
if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty())
{
// There is a race condition between this thread removing the destination
// and another thread queueing a request to this same destination.
// If this destination is removed, but the request queued, a new connection
// will be opened, the exchange will be executed and eventually the connection
// will idle timeout and be closed. Meanwhile a new destination will be created
// in HttpClient and will be used for other requests.
getHttpClient().removeDestination(this);
}
removeDestinationIfIdle();
}
else
{
@ -237,6 +228,27 @@ public abstract class PoolingHttpDestination<C extends Connection> extends HttpD
connectionPool.close();
}
public void abort(Throwable cause)
{
super.abort(cause);
if (getHttpExchanges().isEmpty()) {
removeDestinationIfIdle();
}
}
private void removeDestinationIfIdle() {
if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty())
{
// There is a race condition between this thread removing the destination
// and another thread queueing a request to this same destination.
// If this destination is removed, but the request queued, a new connection
// will be opened, the exchange will be executed and eventually the connection
// will idle timeout and be closed. Meanwhile a new destination will be created
// in HttpClient and will be used for other requests.
getHttpClient().removeDestination(this);
}
}
@Override
public String toString()
{

View File

@ -39,6 +39,7 @@ import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;
@ -280,6 +281,31 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
Assert.assertNotSame(destinationBefore, destinationAfter);
}
@Test
public void testDestinationIsRemovedAfterConnectionError() throws Exception
{
String host = "localhost";
int port = connector.getLocalPort();
client.setRemoveIdleDestinations(true);
Assume.assumeTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty());
server.stop();
Request request = client.newRequest(host, port).scheme(this.scheme);
try
{
request.send();
Assume.assumeTrue("Request to a closed port must fail", false);
} catch (Exception ignored) {
}
long deadline = System.currentTimeMillis() + 100;
while (!client.getDestinations().isEmpty() && System.currentTimeMillis() < deadline)
{
Thread.yield();
}
Assert.assertTrue("Destination must be removed after connection error", client.getDestinations().isEmpty());
}
private Connection timedPoll(Queue<Connection> connections, long time, TimeUnit unit) throws InterruptedException
{
long start = System.nanoTime();