commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 16987] New: - race condition in PoolableConnection.close()
Date Wed, 12 Feb 2003 10:41:22 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16987>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=16987

race condition in PoolableConnection.close()

           Summary: race condition in PoolableConnection.close()
           Product: Commons
           Version: unspecified
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: Major
          Priority: Other
         Component: Dbcp
        AssignedTo: commons-dev@jakarta.apache.org
        ReportedBy: mic@schlund.de


PoolableConnection.close() checks isClosed() but _closed=true is set (later) by
DelegatingConnection.passivate().

It is therefore possible to "close a connection" more than once, numActive gets
decremented more than once, etc.

isClosed() and _closed=true should be run atomically.

I first thought of making the method synchronized, but this could cause
deadlocks, because AdandonedObjectPool.removeAbandoned(), which needs a lock on
the pool object, calls PoolableConnection.close().

I therefore suppose the following patch:


Index: PoolableConnection.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/PoolableConnection.java,v
retrieving revision 1.5
diff -u -r1.5 PoolableConnection.java
--- PoolableConnection.java     1 Nov 2002 15:27:21 -0000       1.5
+++ PoolableConnection.java     12 Feb 2003 10:22:22 -0000
@@ -106,17 +106,19 @@
      * Returns me to my pool.
      */
     public void close() throws SQLException {
-        if(isClosed()) {
-            throw new SQLException("Already closed.");
-        } else {
-            try {
-                _pool.returnObject(this);
-            } catch(SQLException e) {
-                throw e;
-            } catch(RuntimeException e) {
-                throw e;
-            } catch(Exception e) {
-                throw new SQLException(e.toString());
+        synchronized (_pool) {
+            if (isClosed()) {
+                throw new SQLException("Already closed.");
+            } else {
+                try {
+                    _pool.returnObject(this);
+                } catch (SQLException e) {
+                    throw e;
+                } catch (RuntimeException e) {
+                    throw e;
+                } catch (Exception e) {
+                    throw new SQLException(e.toString());
+                }
             }
         }
     }

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message