hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1772396 - in /httpcomponents/httpcore/trunk/httpcore5/src: main/java/org/apache/hc/core5/pool/nio/ test/java/org/apache/hc/core5/pool/nio/
Date Fri, 02 Dec 2016 18:56:30 GMT
Author: olegk
Date: Fri Dec  2 18:56:30 2016
New Revision: 1772396

URL: http://svn.apache.org/viewvc?rev=1772396&view=rev
Log:
Fixed possible connection leak due to cancellation of connection requests

Modified:
    httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/AbstractNIOConnPool.java
    httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/RouteSpecificPool.java
    httpcomponents/httpcore/trunk/httpcore5/src/test/java/org/apache/hc/core5/pool/nio/TestNIOConnPool.java

Modified: httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/AbstractNIOConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/AbstractNIOConnPool.java?rev=1772396&r1=1772395&r2=1772396&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/AbstractNIOConnPool.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/AbstractNIOConnPool.java
Fri Dec  2 18:56:30 2016
@@ -266,6 +266,11 @@ public abstract class AbstractNIOConnPoo
         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();
@@ -280,6 +285,11 @@ public abstract class AbstractNIOConnPoo
         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();
@@ -387,13 +397,19 @@ public abstract class AbstractNIOConnPoo
             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);
+            }
         }
     }
 
@@ -432,9 +448,13 @@ public abstract class AbstractNIOConnPoo
             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);
+                    processNextPendingRequest();
+                }
             } catch (final IOException ex) {
                 pool.failed(request, ex);
             }

Modified: httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/RouteSpecificPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/RouteSpecificPool.java?rev=1772396&r1=1772395&r2=1772396&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/RouteSpecificPool.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore5/src/main/java/org/apache/hc/core5/pool/nio/RouteSpecificPool.java
Fri Dec  2 18:56:30 2016
@@ -145,9 +145,9 @@ abstract class RouteSpecificPool<T, C, E
         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) {

Modified: httpcomponents/httpcore/trunk/httpcore5/src/test/java/org/apache/hc/core5/pool/nio/TestNIOConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore5/src/test/java/org/apache/hc/core5/pool/nio/TestNIOConnPool.java?rev=1772396&r1=1772395&r2=1772396&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore5/src/test/java/org/apache/hc/core5/pool/nio/TestNIOConnPool.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore5/src/test/java/org/apache/hc/core5/pool/nio/TestNIOConnPool.java
Fri Dec  2 18:56:30 2016
@@ -1022,4 +1022,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(NamedEndpoint.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(NamedEndpoint.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