hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1772394 - in /httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src: main/java/org/apache/http/nio/pool/ test/java/org/apache/http/nio/pool/
Date Fri, 02 Dec 2016 18:53:34 GMT
Author: olegk
Date: Fri Dec  2 18:53:34 2016
New Revision: 1772394

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

Modified:
    httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
    httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
    httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java

Modified: httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java?rev=1772394&r1=1772393&r2=1772394&view=diff
==============================================================================
--- httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
(original)
+++ httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/AbstractNIOConnPool.java
Fri Dec  2 18:53:34 2016
@@ -43,8 +43,8 @@ import java.util.concurrent.atomic.Atomi
 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 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();
@@ -341,6 +346,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();
@@ -450,13 +460,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);
+            }
         }
     }
 
@@ -495,9 +511,15 @@ 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);
+                    if (this.ioreactor.getStatus().compareTo(IOReactorStatus.ACTIVE) <=
0) {
+                        processNextPendingRequest();
+                    }
+                }
             } catch (final IOException ex) {
                 pool.failed(request, ex);
             }

Modified: httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java?rev=1772394&r1=1772393&r2=1772394&view=diff
==============================================================================
--- httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
(original)
+++ httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/main/java/org/apache/http/nio/pool/RouteSpecificPool.java
Fri Dec  2 18:53:34 2016
@@ -148,9 +148,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/branches/4.4.x/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java?rev=1772394&r1=1772393&r2=1772394&view=diff
==============================================================================
--- httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
(original)
+++ httpcomponents/httpcore/branches/4.4.x/httpcore-nio/src/test/java/org/apache/http/nio/pool/TestNIOConnPool.java
Fri Dec  2 18:53:34 2016
@@ -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