hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject [07/50] httpcomponents-core git commit: Fixed possible connection leak due to cancellation of connection requests
Date Tue, 09 May 2017 20:01:33 GMT
Fixed possible connection leak due to cancellation of connection requests

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpcore/branches/4.4.x@1772394
13f79535-47bb-0310-9956-ffa450edef68


Project: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/commit/141eacdb
Tree: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/tree/141eacdb
Diff: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/diff/141eacdb

Branch: refs/heads/4.4.x
Commit: 141eacdba429c0e37c7f4477a49974c96f4c7c83
Parents: 2af8359
Author: Oleg Kalnichevski <olegk@apache.org>
Authored: Fri Dec 2 18:53:34 2016 +0000
Committer: Oleg Kalnichevski <olegk@apache.org>
Committed: Fri Dec 2 18:53:34 2016 +0000

----------------------------------------------------------------------
 .../http/nio/pool/AbstractNIOConnPool.java      | 32 +++++++--
 .../apache/http/nio/pool/RouteSpecificPool.java |  4 +-
 .../apache/http/nio/pool/TestNIOConnPool.java   | 68 +++++++++++++++++++-
 3 files changed, 96 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/141eacdb/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
----------------------------------------------------------------------
diff --git a/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
b/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
index 9cae419..02533ba 100644
--- a/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
+++ b/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
@@ -43,8 +43,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
-import org.apache.http.annotation.ThreadingBehavior;
 import org.apache.http.annotation.Contract;
+import org.apache.http.annotation.ThreadingBehavior;
 import org.apache.http.concurrent.BasicFuture;
 import org.apache.http.concurrent.FutureCallback;
 import org.apache.http.nio.reactor.ConnectingIOReactor;
@@ -327,6 +327,11 @@ public abstract class AbstractNIOConnPool<T, C, E extends PoolEntry<T,
C>>
         final ListIterator<LeaseRequest<T, C, E>> it = this.leasingRequests.listIterator();
         while (it.hasNext()) {
             final LeaseRequest<T, C, E> request = it.next();
+            final BasicFuture<E> future = request.getFuture();
+            if (future.isCancelled()) {
+                it.remove();
+                continue;
+            }
             final boolean completed = processPendingRequest(request);
             if (request.isDone() || completed) {
                 it.remove();
@@ -341,6 +346,11 @@ public abstract class AbstractNIOConnPool<T, C, E extends PoolEntry<T,
C>>
         final ListIterator<LeaseRequest<T, C, E>> it = this.leasingRequests.listIterator();
         while (it.hasNext()) {
             final LeaseRequest<T, C, E> request = it.next();
+            final BasicFuture<E> future = request.getFuture();
+            if (future.isCancelled()) {
+                it.remove();
+                continue;
+            }
             final boolean completed = processPendingRequest(request);
             if (request.isDone() || completed) {
                 it.remove();
@@ -450,13 +460,19 @@ public abstract class AbstractNIOConnPool<T, C, E extends PoolEntry<T,
C>>
             final BasicFuture<E> future = request.getFuture();
             final Exception ex = request.getException();
             final E result = request.getResult();
+            boolean successfullyCompleted = false;
             if (ex != null) {
                 future.failed(ex);
             } else if (result != null) {
-                future.completed(result);
+                if (future.completed(result)) {
+                    successfullyCompleted = true;
+                }
             } else {
                 future.cancel();
             }
+            if (!successfullyCompleted) {
+                release(result, true);
+            }
         }
     }
 
@@ -495,9 +511,15 @@ public abstract class AbstractNIOConnPool<T, C, E extends PoolEntry<T,
C>>
             try {
                 final C conn = this.connFactory.create(route, session);
                 final E entry = pool.createEntry(request, conn);
-                this.leased.add(entry);
-                pool.completed(request, entry);
-                onLease(entry);
+                if (pool.completed(request, entry)) {
+                    this.leased.add(entry);
+                    onLease(entry);
+                } else {
+                    this.available.add(entry);
+                    if (this.ioreactor.getStatus().compareTo(IOReactorStatus.ACTIVE) <=
0) {
+                        processNextPendingRequest();
+                    }
+                }
             } catch (final IOException ex) {
                 pool.failed(request, ex);
             }

http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/141eacdb/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
----------------------------------------------------------------------
diff --git a/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java b/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
index eb9fa0f..7e9a240 100644
--- a/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
+++ b/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
@@ -148,9 +148,9 @@ abstract class RouteSpecificPool<T, C, E extends PoolEntry<T, C>>
{
         return entry;
     }
 
-    public void completed(final SessionRequest request, final E entry) {
+    public boolean completed(final SessionRequest request, final E entry) {
         final BasicFuture<E> future = removeRequest(request);
-        future.completed(entry);
+        return future.completed(entry);
     }
 
     public void cancelled(final SessionRequest request) {

http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/141eacdb/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
----------------------------------------------------------------------
diff --git a/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java b/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
index be0d2f7..859d547 100644
--- a/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
+++ b/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
@@ -27,9 +27,9 @@
 package org.apache.http.nio.pool;
 
 import java.io.IOException;
+import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
-import java.net.ConnectException;
 import java.net.UnknownHostException;
 import java.util.Collections;
 import java.util.concurrent.ExecutionException;
@@ -1023,4 +1023,70 @@ public class TestNIOConnPool {
         pool.requestTimeout(Mockito.mock(SessionRequest.class));
     }
 
+    @Test
+    public void testLeaseRequestCanceled() throws Exception {
+        final IOSession iosession1 = Mockito.mock(IOSession.class);
+        Mockito.when(iosession1.isClosed()).thenReturn(Boolean.TRUE);
+        final SessionRequest sessionRequest1 = Mockito.mock(SessionRequest.class);
+        Mockito.when(sessionRequest1.getAttachment()).thenReturn("somehost");
+        Mockito.when(sessionRequest1.getSession()).thenReturn(iosession1);
+
+        final ConnectingIOReactor ioreactor = Mockito.mock(ConnectingIOReactor.class);
+        Mockito.when(ioreactor.connect(
+                Mockito.any(SocketAddress.class), Mockito.any(SocketAddress.class),
+                Mockito.any(), Mockito.any(SessionRequestCallback.class))).
+                thenReturn(sessionRequest1);
+        Mockito.when(ioreactor.getStatus()).thenReturn(IOReactorStatus.ACTIVE);
+
+        final LocalSessionPool pool = new LocalSessionPool(ioreactor, 1, 1);
+
+        final Future<LocalPoolEntry> future1 = pool.lease("somehost", null, 0, TimeUnit.MILLISECONDS,
null);
+        future1.cancel(true);
+
+        pool.requestCompleted(sessionRequest1);
+
+        Assert.assertTrue(future1.isDone());
+        final LocalPoolEntry entry1 = future1.get();
+        Assert.assertNull(entry1);
+
+        final PoolStats totals = pool.getTotalStats();
+        Assert.assertEquals(1, totals.getAvailable());
+        Assert.assertEquals(0, totals.getLeased());
+    }
+
+    @Test
+    public void testLeaseRequestCanceledWhileConnecting() throws Exception {
+        final IOSession iosession1 = Mockito.mock(IOSession.class);
+        Mockito.when(iosession1.isClosed()).thenReturn(Boolean.TRUE);
+        final SessionRequest sessionRequest1 = Mockito.mock(SessionRequest.class);
+        Mockito.when(sessionRequest1.getAttachment()).thenReturn("somehost");
+        Mockito.when(sessionRequest1.getSession()).thenReturn(iosession1);
+
+        final ConnectingIOReactor ioreactor = Mockito.mock(ConnectingIOReactor.class);
+        Mockito.when(ioreactor.connect(
+                Mockito.any(SocketAddress.class), Mockito.any(SocketAddress.class),
+                Mockito.any(), Mockito.any(SessionRequestCallback.class))).
+                thenReturn(sessionRequest1);
+        Mockito.when(ioreactor.getStatus()).thenReturn(IOReactorStatus.ACTIVE);
+
+        final LocalSessionPool pool = new LocalSessionPool(ioreactor, 1, 1);
+
+        final Future<LocalPoolEntry> future1 = pool.lease("somehost", null, 0, TimeUnit.MILLISECONDS,
null);
+
+        pool.requestCompleted(sessionRequest1);
+
+        Assert.assertTrue(future1.isDone());
+        final LocalPoolEntry entry1 = future1.get();
+        Assert.assertNotNull(entry1);
+
+        final Future<LocalPoolEntry> future2 = pool.lease("somehost", null, 0, TimeUnit.MILLISECONDS,
null);
+        future2.cancel(true);
+
+        pool.release(entry1, true);
+
+        final PoolStats totals = pool.getTotalStats();
+        Assert.assertEquals(1, totals.getAvailable());
+        Assert.assertEquals(0, totals.getLeased());
+    }
+
 }


Mime
View raw message