Daniel, here is a patch for the bug specifically:
diff -u -r1.4 PoolBrokerService.java
--- PoolBrokerService.java 2000/09/19 17:17:08 1.4
+++ PoolBrokerService.java 2000/09/21 08:55:41
@@ -126,11 +126,11 @@
protected Hashtable dbMaps = null;
/**
- * Consumers should always employ the <code>getInstance()</code>
+ * Consumers should always employ the <code>getInstance()</code>
* method to get the single instance of this class.
*
- * Note that this ctor should probably be switched back to private when
- * its deprecated extension in the
+ * Note that this ctor should probably be switched back to private when
+ * its deprecated extension in the
* <code>org.apache.turbine.util.db.pool</code> package is removed.
*/
public PoolBrokerService()
@@ -206,19 +206,19 @@
url = url.trim();
// Quick (non-sync) check for the pool we want.
- pool = (ConnectionPool) pools.get(url);
+ pool = (ConnectionPool) pools.get(url + username);
if ( pool == null )
{
// Pool not there...
synchronized ( pools )
{
// ... sync and look again to avoid race collisions.
- pool = (ConnectionPool) pools.get(url);
+ pool = (ConnectionPool) pools.get(url + username);
if ( pool == null )
{
// Still not there. Create and add.
pool = new ConnectionPool();
- pools.put( url, pool );
+ pools.put( url + username, pool );
}
}
}
@@ -331,12 +331,12 @@
String url = TurbineResources.getString( "database." +
name +
".url", "" ).trim();
- ConnectionPool cp = (ConnectionPool)pools.get(url);
+ ConnectionPool cp = (ConnectionPool)pools.get(url + username);
if ( cp == null)
{
DBConnection dbcon = getConnection(name);
releaseConnection(dbcon);
- cp = (ConnectionPool)pools.get(url);
+ cp = (ConnectionPool)pools.get(url + username);
}
return cp.getDB();
}
@@ -378,10 +378,10 @@
ConnectionPool pool = null;
synchronized ( pools )
{
- pool = (ConnectionPool) pools.get( url );
+ pool = (ConnectionPool) pools.get( url + username);
}
- // If there is a ConnecitonPool associated with this
+ // If there is a ConnecitonPool associated with this
// connection's URL, release the connection from that pool.
if ( pool != null )
{
*****CVS exited normally with code 1*****
I have some other suggestions for the pool:
1) The stuff I mentioned in my last post about the
initialization of the connection pools
2) Some solutions to the problem of leaks from the pool
(mainly logging, but also the possibility of returning
a DBConnection to a pool before it is finalized)
3) Fiddling with Jon's run() function in ConnectionPool,
to enable some constant monitoring, and logging the
status of the pool at a specified interval.
I'll submit some patches for this, but comments on how my
ideas might be received would be appreciated.
> -----Original Message-----
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED]]On Behalf Of Daniel L. Rall
> Sent: 21. september 2000 00:11
> To: Turbine
> Subject: Re: Connection Pooling - Bug
>
>
> Magnus, without looking at the code or API, I agree with what you've got
> mentioned here. I will examine it more closely as time allows. Patches
> welcome, of course. ;)
>
> Magn�s ��r Torfason wrote:
> >
> > It seems to me that there must be a bug in the interaction between the
> > PoolBrokerService and the ConnectionPool:
> >
> > public DBConnection getConnection(
> > String driver,
> > String url,
> > String username,
> > String password )
> > {
> > ...
> > pool = (ConnectionPool) pools.get(url);
> > ...
> > return pool.getConnection(driver, url, username, password);
> > }
> >
> > The pools are indexed by url only, NOT by username and password.
> > I also checked if pool.getConnection(driver,url,username,password) would
> > possibly handle this, but they don't. What happens is that if
> a connection
> > for another username exists, it (a connection with the
> incorrect user) is
> > returned.
> >
> > This can be fixed by adding the username to the key of the
> pools Hashtable
> > (driver and password may safely be left out):
> >
> > pool = (ConnectionPool) pools.get(url + username);
> >
> > <MyTwoCents>
> >
> > What bothers me is that the ConnectionPool.getConnection(...)
> method even
> > takes the database parameters. If instead the creator took the
> parameters
> > and saved them as instance variables, and the getConnection
> function took no
> > parameters, this bug would probably not have been introduced to
> the system.
> >
> > A similar thing applies to the PoolBrokerService itself. Why
> isn't there a
> > method like this in there:
> >
> > public void registerPool(String poolname, String driver, String
> url, String
> > username, String password)
> >
> > The PoolBrokerService.getConnection(...) method would then not
> need to take
> > the database parameters, only the name of the pool. This would be a lot
> > cleaner. I have nothing to gain from having
> driver/url/username/password
> > accessible everywhere in the system, much better to give each pool a
> > meaningful name, and reference it by that.
> >
> > The modifications are minor, and I can submit a set of patches. The old
> > functions can even stay, one could choose which version to use.
> >
> > </MyTwoCents>
> >
> > More to come, on a related subject.
>
> --
>
> Daniel Rall <[EMAIL PROTECTED]>
>
>
> ------------------------------------------------------------
> To subscribe: [EMAIL PROTECTED]
> To unsubscribe: [EMAIL PROTECTED]
> Search: <http://www.mail-archive.com/turbine%40list.working-dogs.com/>
> Problems?: [EMAIL PROTECTED]
>
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Search: <http://www.mail-archive.com/turbine%40list.working-dogs.com/>
Problems?: [EMAIL PROTECTED]
