I'm absolutely new to open-source development and groups like this, so
forgive me if I'm not following the correct protocol, but I found and
fixed a bug in DBCP and would like to contribute my patch back to the
project. I've created a patch file and attached it to this e-mail. If
a comitter to commons-dbcp would please look at my patch as well as the
explanation below, that'd be great. If you'd like me to do anything
differently next time I have a patch to offer, just let me know :-)
Wayne Woodfield
-------------------------
BUG DESCRIPTION:
I discovered this bug while using Tomcat DBCP. I was using
removeAbandoned=true, so AbandonedObjectPool was used. We had
maxActive=10 and maxIdle=2. Under a really heavy load, numActive gets
off. Over time, well over a hundred connections to the database get
spawned at the same time. Because numActive gets so far off, it seems
like the maxActive limit is ignored. The database runs out of
connections to give. After the heavy load has passed and things go back
to normal, a call to DataSource.getNumActive() shows that numActive is
negative.
CAUSE:
I was able to trace this problem back to a bug in AbandonedObjectPool.
AbandonedObjectPool initiates an abandoned connection check every time
someone tries to borrow a connection from an exhausted or
nearly-exhausted connection pool. That means that several threads could
initiate an abandoned connection sweep at once. If two abandoned
connection checks running at the same time try to invalidate the same
connection, they will both be allowed to do so. GenericObjectPool
(where numActive is stored) doesn't have any way of knowing whether a
connection has been invalidated before. It just decrements numActive
again. A connection can also be invalidated and returned to the idle
pool at the same time, again decrementing numActive twice.
AbandonedObjectPool keeps track of active connections. When a
connection is returned or invalidated, the connection is removed from
the active list. AbandonedObjectPool should check to see whether the
connection is still in the active list before removing it, and it
doesn't.
SOLUTION:
In AbandonedObjectPool's returnObject(Object obj) and
invalidateObject(Object obj) methods, I've added a few lines to ensure
that connections cannot be invalidated or returned twice. Before
invalidating a connection or returning a connection to the idle pool,
I'm simply checking to see whether the connection to return or
invalidate is still in the active connection list. If it isn't, then
this connection has already been invalidated, and the return or
invalidation is aborted.
I took advantage of the fact that calling remove(Object) on an ArrayList
returns true if the object to remove was actually found, and false
otherwise, so I didn't even have to do an additional traversal of the
active connection list to find out whether the connection had already
been removed, so there is practically no performance impact at all :-)
See the attached patch.
? build
? patchfile.txt
Index: src/java/org/apache/commons/dbcp/AbandonedObjectPool.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/dbcp/src/java/org/apache/commons/dbcp/AbandonedObjectPool.java,v
retrieving revision 1.13
diff -u -r1.13 AbandonedObjectPool.java
--- src/java/org/apache/commons/dbcp/AbandonedObjectPool.java 28 Feb 2004 11:48:04
-0000 1.13
+++ src/java/org/apache/commons/dbcp/AbandonedObjectPool.java 19 Apr 2004 03:46:44
-0000
@@ -91,7 +91,10 @@
public void returnObject(Object obj) throws Exception {
if (config != null && config.getRemoveAbandoned()) {
synchronized(trace) {
- trace.remove(obj);
+ boolean foundObject = trace.remove(obj);
+ if (!foundObject) {
+ return; // This connection has already been
invalidated. Stop now.
+ }
}
}
super.returnObject(obj);
@@ -100,7 +103,10 @@
public void invalidateObject(Object obj) throws Exception {
if (config != null && config.getRemoveAbandoned()) {
synchronized(trace) {
- trace.remove(obj);
+ boolean foundObject = trace.remove(obj);
+ if (!foundObject) {
+ return; // This connection has already been
returned or invalidated. Stop now.
+ }
}
}
super.invalidateObject(obj);
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
