commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Waldhoff, Rodney" <rwald...@us.britannica.com>
Subject RE: [POOL] Bug
Date Tue, 02 Apr 2002 22:52:45 GMT
To clarify, I think it's pretty much as simple as this:

public class CheckingObjectPool extends GenericObjectPool {
  public void returnObject(Object obj) throws Exception {
    if(_pool.contains(obj)) {
       throw new Exception(obj + " is already in the pool.");
    }
  }
}

to extend GenericObjectPool with this behavior.

-----Original Message-----
From: Waldhoff, Rodney [mailto:rwaldhof@us.britannica.com]
Sent: Monday, April 01, 2002 12:25 PM
To: 'Jakarta Commons Developers List'
Subject: RE: [POOL] Bug


> This could easily be done by doing a "contains()" 
> check on the collection that backs the pool before 
> adding the object to it.

If and only if the equality method identifies unique instances of objects
within the pool.

You could implement a [Keyed]ObjectPool to do this, or perhaps better still,
implement it via the passivateObject method of [Keyed]PoolableObjectFactory.
Then it would be tied to the top of object being pooled, when you could
enforce a meaningful contains() method.

I'm not sure it makes sense to do this check in the general case, since, as
you note:

> if there is code that returns the object to the pool more 
> than once, then that code itself is buggy, 

What do the rest of you think?

-----Original Message-----
From: James House [mailto:jhouse@part.net]
Sent: Monday, April 01, 2002 10:26 AM
To: commons-dev@jakarta.apache.org
Subject: [POOL] Bug



I've ran into another bug with Pool / DBCP.

The problem lies in GenericObjectPool's returnObject() method.

There is no check to see if the object has already been returned, it simply 
adds the object back into the pool.

If the object has already been returned, then the object now appears two or 
more times in the pool.  This leads to serious problems.  For one, it leads 
to multiple threads "borrowing" the same object at the same time.  It also 
makes the pool "fill up" which causes very serious problems -- once the 
maxIdle pool count is reached (which happens quickly in this scenario), the 
pool starts "tossing" out objects that are being returned.  It does so by 
calling "destroyObject".  The problem with this is that the object (in this 
case, a DB connection) gets destroyed, but there are multiple pointers to 
it still in the pool!  - the next time the object is borrowed from the pool 
it is completely invalid! (the connection has been closed).

I understand that if there is code that returns the object to the pool more 
than once, then that code itself is buggy, however I think there should 
also be real safeguards in the pool (returnObject()) that disallow the same 
object to be in the pool multiple times.  This could easily be done by 
doing a "contains()" check on the collection that backs the pool before 
adding the object to it.  If the object is found to already be in the pool, 
an exception should be thrown so that the code utilizing the pool can be 
corrected.  Perhaps "contains()" is not the most efficient way to do the 
check (if the pool is large) -- but it's probably the most quick and easy 
way to make the fix, as it simply adds 2 lines of code:  a line to test the 
condition, and another line to throw an exception.

James


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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message