hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1382442 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java
Date Sun, 09 Sep 2012 10:41:41 GMT
Author: olegk
Date: Sun Sep  9 10:41:41 2012
New Revision: 1382442

URL: http://svn.apache.org/viewvc?rev=1382442&view=rev
Log:
HTTPCLIENT-1229: Fixed NPE in BasicClientConnectionManager that can be triggered by releasing
connection after the connection manager has already been shut down

Modified:
    httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
    httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java

Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=1382442&r1=1382441&r2=1382442&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Sun Sep  9 10:41:41 2012
@@ -1,6 +1,10 @@
 Changes since 4.2.1 
 -------------------
 
+* [HTTPCLIENT-1229] Fixed NPE in BasicClientConnectionManager that can be triggered by releasing
+  connection after the connection manager has already been shut down. 
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * [HTTPCLIENT-1227] Date parsing in DateUtils made more efficient.
   Contributed by Patrick Linskey <pcl at apache.org>
 

Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java?rev=1382442&r1=1382441&r2=1382442&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java
(original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/conn/BasicClientConnectionManager.java
Sun Sep  9 10:41:41 2012
@@ -33,6 +33,7 @@ import java.util.concurrent.atomic.Atomi
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.http.HttpClientConnection;
 import org.apache.http.annotation.GuardedBy;
 import org.apache.http.annotation.ThreadSafe;
 import org.apache.http.conn.ClientConnectionManager;
@@ -152,11 +153,11 @@ public class BasicClientConnectionManage
         if (route == null) {
             throw new IllegalArgumentException("Route may not be null.");
         }
-        assertNotShutdown();
-        if (this.log.isDebugEnabled()) {
-            this.log.debug("Get connection for route " + route);
-        }
         synchronized (this) {
+            assertNotShutdown();
+            if (this.log.isDebugEnabled()) {
+                this.log.debug("Get connection for route " + route);
+            }
             if (this.conn != null) {
                 throw new IllegalStateException(MISUSE_MESSAGE);
             }
@@ -179,17 +180,26 @@ public class BasicClientConnectionManage
         }
     }
 
+    private void shutdownConnection(final HttpClientConnection conn) {
+        try {
+            conn.shutdown();
+        } catch (IOException iox) {
+            if (this.log.isDebugEnabled()) {
+                this.log.debug("I/O exception shutting down connection", iox);
+            }
+        }
+    }
+    
     public void releaseConnection(final ManagedClientConnection conn, long keepalive, TimeUnit
tunit) {
-        assertNotShutdown();
         if (!(conn instanceof ManagedClientConnectionImpl)) {
             throw new IllegalArgumentException("Connection class mismatch, " +
                  "connection not obtained from this manager");
         }
-        if (this.log.isDebugEnabled()) {
-            this.log.debug("Releasing connection " + conn);
-        }
         ManagedClientConnectionImpl managedConn = (ManagedClientConnectionImpl) conn;
         synchronized (managedConn) {
+            if (this.log.isDebugEnabled()) {
+                this.log.debug("Releasing connection " + conn);
+            }
             if (managedConn.getPoolEntry() == null) {
                 return; // already released
             }
@@ -198,15 +208,13 @@ public class BasicClientConnectionManage
                 throw new IllegalStateException("Connection not obtained from this manager");
             }
             synchronized (this) {
+                if (this.shutdown) {
+                    shutdownConnection(managedConn);
+                    return;
+                }
                 try {
                     if (managedConn.isOpen() && !managedConn.isMarkedReusable())
{
-                        try {
-                            managedConn.shutdown();
-                        } catch (IOException iox) {
-                            if (this.log.isDebugEnabled()) {
-                                this.log.debug("I/O exception shutting down released connection",
iox);
-                            }
-                        }
+                        shutdownConnection(managedConn);
                     }
                     this.poolEntry.updateExpiry(keepalive, tunit != null ? tunit : TimeUnit.MILLISECONDS);
                     if (this.log.isDebugEnabled()) {
@@ -230,8 +238,8 @@ public class BasicClientConnectionManage
     }
 
     public void closeExpiredConnections() {
-        assertNotShutdown();
         synchronized (this) {
+            assertNotShutdown();
             long now = System.currentTimeMillis();
             if (this.poolEntry != null && this.poolEntry.isExpired(now)) {
                 this.poolEntry.close();
@@ -244,8 +252,8 @@ public class BasicClientConnectionManage
         if (tunit == null) {
             throw new IllegalArgumentException("Time unit must not be null.");
         }
-        assertNotShutdown();
         synchronized (this) {
+            assertNotShutdown();
             long time = tunit.toMillis(idletime);
             if (time < 0) {
                 time = 0;
@@ -259,8 +267,8 @@ public class BasicClientConnectionManage
     }
 
     public void shutdown() {
-        this.shutdown = true;
         synchronized (this) {
+            this.shutdown = true;
             try {
                 if (this.poolEntry != null) {
                     this.poolEntry.close();



Mime
View raw message