I'd like to run this by everyone again.  I'm going to be looking at it
in depth tonight, but at a cursory glance it looks solid.  If I don't
hear any complaints or notice any gotchas, I'm going to be merging it
in.
-- 

Daniel Rall <[EMAIL PROTECTED]>
http://collab.net/ | open source | do the right thing




I have been doing some work on the connection pooling framework.  The
patches are rather lengthy, since I've added quite a bit, but I believe
everything is backward compatible.

The files I changed were:
PoolBrokerService.java
ConnectionPool.java
DBConnection.java

The stuff I have changed:

DBConnection.java
-----------------
Added awareness of the username for each instance

Added awareness of the pool the instance came from

Added a finalizer, that will return the connection back
to the pool it came from, preventing leakage from the
pool.  It will also log that a connection was leaked.



ConnectionPool.java
-------------------
Added functionality to getConnection() and releaseConnection()
to link a DBConnection to the given pool, enabling it
to prevent leakage.

Added Constructors containing database parameters,
and added getConnection() functions without them,
because it is the logical way to use a connection pool

Added instance variables to store the database parameters,
so that they do not have to be passed each time a connection
is gotten from the pool


PoolBrokerService.java
----------------------
Added a method, registerPool() to handle the creation of
pools in a single place, and enable the registration of
pools dynamically in code.

Modified getConnection() methods so that they call the
ConnectionPool.getConnection() methods without passing
database parameters.

Added methods to get a pool from the service.
Reasoning: The ConnectionPool is thread safe, so there
is no safety reason to prevent direct access to the
pools.  (I have some classes that expect to be passed
an instance of a connection pool, and this is needed for
them)





I also changed the names of DBConnection objects from
connection to dbconn, do distinguish them from the
java.sql.Connection objects that are being thrown
around, and were also in many cases called connection






diff -u -r1.5 DBConnection.java
--- DBConnection.java   2000/09/22 01:16:27     1.5
+++ DBConnection.java   2000/09/25 16:36:56
@@ -78,12 +78,21 @@
  */
 public class DBConnection
 {
+    /** The ConnectionPool that this DBConnection came from.
+     *  A null means that the DBConnection either does not
+     *  belong to any pool, or that it is currently inside it's pool
+     */
+    private ConnectionPool pool = null;
+
     /** The JDBC database connection. */
     private Connection connection = null;

     /** The URL of this connection. */
     private String url = null;

+    /** The user name for this connection. */
+    private String username = null;
+
     /**
      * The time in milliseconds at which the connection was
      * created.
@@ -91,6 +100,17 @@
     private long timestamp;

     /**
+     * Creates a Turbine <code>DBConnection</code> specifying
+     * nothing but a single database Connection.
+     *
+     * @param connection The JDBC connection to wrap.
+     */
+    protected DBConnection(Connection connection)
+    {
+        this(connection, null, null, null);
+    }
+
+    /**
      * Creates a Turbine <code>DBConnection</code> with the specified
      * attributes.
      *
@@ -99,12 +119,103 @@
      */
     protected DBConnection(Connection connection, String url)
     {
+        this(connection, url, null, null);
+    }
+
+    /**
+     * Creates a Turbine <code>DBConnection</code> with the specified
+     * attributes.
+     *
+     * @param connection The JDBC connection to wrap.
+     * @param url The URL we're connecting to.
+     * @param username The user name we are connecting as
+     */
+    protected DBConnection(Connection connection, String url, String
username)
+    {
+        this(connection, url, username, null);
+    }
+
+    /**
+     * Creates a Turbine <code>DBConnection</code> that is part of
+     * a pool.
+     *
+     * @param connection The JDBC connection to wrap.
+     * @param url The URL we're connecting to.
+     * @param username The user name we are connecting as
+     * @param pool The ConnectionPool that this DBConnection belongs to
+     */
+    protected DBConnection(Connection connection, String url, String
username, ConnectionPool pool)
+    {
         this.connection = connection;
         this.url = url;
+        this.username = username;
+        this.pool = pool;
         this.timestamp = System.currentTimeMillis();
     }

     /**
+     * The finalizer helps prevent leakage from ConnectionPools
+        */
+    protected void finalize() throws Throwable {
+               if (pool != null) {
+                       // If this DBConnection object is finalized
while linked
+                       // to a ConnectionPool, it means that it was
taken from a pool
+                       // and not returned.  We log this fact, close
the underlying
+                       // Connection, and return it to the
ConnectionPool.
+                       Log.warn (      "A DBConnection was finalized,
without being returned "
+                                       +       "to the ConnectionPool
it belonged to" );
+
+                       // Closing the Connection ensures that if anyone
tries to use it,
+                       // an error will occur.
+                       connection.close();
+
+                       // Releasing a new DBConnection object will
prevent leaks
+                       // from the pool
+                       pool.releaseConnection(new
DBConnection(connection,url,username,pool));
+               }
+       }
+
+
+       /**
+        * Links this DBConnection with a ConnectionPool.
+        *
+        * @param The pool to link to
+        */
+       protected void link(ConnectionPool pool)
+       {
+               if (pool == null) {
+                       throw new NullPointerException("Cannot link to a
null pool");
+               }
+               this.pool = pool;
+       }
+
+       /**
+        * Unlink the DBConnection from it's pool.
+        * @param The pool to unlink from
+        * @exception Thrown if someone tries to unlink from another
pool
+        */
+       protected void unlink(ConnectionPool pool) throws Exception
+       {
+               if ( (this.pool != pool) && (pool != null) ) {
+                       throw new NullPointerException("Trying to unlink
from the wrong pool");
+               }
+               this.pool = null;
+       }
+
+       /**
+        * Returns the pool this DBConnection came from, or null if
+        * it is not linked to any pool
+        *
+        * @return The pool the DBConnection came from
+        */
+       public ConnectionPool getPool()
+       {
+               return pool;
+       }
+
+
+
+    /**
      * Returns a JDBC connection.
      *
      * @return The database connection.
@@ -134,6 +245,16 @@
     public String getUrl()
     {
         return url;
+    }
+
+    /**
+     * Returns the connection user name.
+     *
+     * @return A String with the connection username.
+     */
+    public String getUsername()
+    {
+        return username;
     }

     /**











diff -u -r1.10 ConnectionPool.java
--- ConnectionPool.java 2000/09/18 05:24:45     1.10
+++ ConnectionPool.java 2000/09/25 16:40:58
@@ -85,6 +85,26 @@
      */
     private Stack pool = null;

+       /**
+        * The driver type for this pool.
+        */
+       private String driver = null;
+
+       /**
+        * The url for this pool.
+        */
+       private String url = null;
+
+       /**
+        * The user name for this pool.
+        */
+       private String username = null;
+
+       /**
+        * The password for this pool.
+        */
+       private String password = null;
+
     /**
      * The current number of database connections that have been
      * created.
@@ -137,31 +157,84 @@
     /**
      * Creates a <code>ConnectionPool</code> with the default
      * attributes.
+     * @deprecated Use the constructor specifying dp parameters
      */
     public ConnectionPool()
     {
+        this(null,null,null,null);
+    }
+
+    /**
+     * Creates a <code>ConnectionPool</code> with the specified
+     * attributes.
+     *
+     * @param maxCons The maximum number of connections for this pool.
+     * @param expiryTime The expiration time in milliseconds.
+     * @deprecated Use the constructor specifying dp parameters
+     */
+    public ConnectionPool(int maxCons, long expiryTime)
+    {
+               this(null,null,null,null,maxCons,expiryTime);
+    }
+
+    /**
+     * Creates a <code>ConnectionPool</code> with the default
+     * attributes.
+        *
+        * @param driver The driver type for this pool.
+        * @param url The url for this pool.
+        * @param usernam The user name for this pool.
+        * @param password The password for this pool.
+     */
+    public ConnectionPool(String driver,
+                          String url,
+                          String username,
+                          String password)
+    {
         pool = new Stack();
-        maxConnections =
TurbineResources.getInt("database.maxConnections",
-                                                 10);
-        expiryTime = TurbineResources.getLong("database.expiryTime",
3600000);
+
+        maxConnections =
+               TurbineResources.getInt("database.maxConnections", 10);
+        expiryTime =
+               TurbineResources.getLong("database.expiryTime",
3600000);
         maxConnectionAttempts =
             TurbineResources.getLong("database.maxConnectionAttempts",
50);
-        connectionWaitTimeout =
-            TurbineResources.getLong("database.connectionWaitTimeout",
+        connectionWaitTimeout =
+            TurbineResources.getLong("database.connectionWaitTimeout",
                                      10 * 1000);
+
+               this.driver = driver;
+               this.url = url;
+               this.username = username;
+               this.password = password;
     }

     /**
      * Creates a <code>ConnectionPool</code> with the specified
      * attributes.
      *
+        * @param driver The driver type for this pool.
+        * @param url The url for this pool.
+        * @param usernam The user name for this pool.
+        * @param password The password for this pool.
      * @param maxCons The maximum number of connections for this pool.
+     * @param maxCons The maximum number of connections for this pool.
      * @param expiryTime The expiration time in milliseconds.
      */
-    public ConnectionPool(int maxCons, long expiryTime)
+    public ConnectionPool(String driver,
+                          String url,
+                          String username,
+                          String password,
+                                                 int maxCons,
+                                                 long expiryTime)
     {
         pool = new Stack();

+               this.driver = driver;
+               this.url = url;
+               this.username = username;
+               this.password = password;
+
         this.maxConnections = maxCons;
         this.expiryTime = expiryTime;
     }
@@ -178,9 +251,10 @@
         shutdown();
     }

-    /**
-     * Returns a connection.
-     *
+       /**
+        * Returns a connection that maintains a link to the pool
+        * it came from.
+        *
      * @param driver The fully-qualified name of the JDBC driver to
use.
      * @param url The URL of the database from which the connection is
      * desired.
@@ -189,23 +263,54 @@
      * @return A database connection.
      * @exception Exception.
      */
-    public synchronized DBConnection getConnection(String driver,
-                                                   String url,
-                                                   String username,
-                                                   String password)
+    public synchronized DBConnection getConnection()
         throws Exception
     {
-        DBConnection connection = null;
+        DBConnection dbconn = null;

         if ( pool.empty() && totalConnections < maxConnections )
         {
-            connection = getNewConnection(driver, url, username,
password);
+            dbconn = getNewConnection();
         }
         else
         {
-            connection = getPooledConnection(driver, url, username,
password);
+            dbconn = getPooledConnection();
         }
-        return connection;
+        dbconn.link(this);
+        return dbconn;
+       }
+
+
+    /**
+     * This function is unsafe to use, since the user passed parameters
+     * that he has no way to know if are used.
+     *
+     * @param driver The fully-qualified name of the JDBC driver to
use.
+     * @param url The URL of the database from which the connection is
+     * desired.
+     * @param username The name of the database user.
+     * @param password The password of the database user.
+     * @return A database connection.
+     * @deprecated Database parameters should not be specified each
+     * time a DBConnection is fetched from the pool.
+     * @exception Exception.
+     */
+    public synchronized DBConnection getConnection(String driver,
+                                                   String url,
+                                                   String username,
+                                                   String password)
+        throws Exception
+    {
+               if ( (this.driver == null) && (this.url == null) &&
+                        (this.username == null) && (this.password ==
null) )
+               {
+                       this.driver = driver;
+                       this.url = url;
+                       this.username = username;
+                       this.password = password;
+               }
+
+               return getConnection();
     }

     /**
@@ -225,22 +330,14 @@
      * information by <code>url</code>, <code>username</code>, and
      * <code>password</code>.
      *
-     * @param driver The fully-qualified name of the JDBC driver to
use.
-     * @param url The URL of the database from which the connection is
-     * desired.
-     * @param username The name of the database user.
-     * @param password The password of the database user.
      * @return A database connection.
      * @exception Exception
      */
-    protected DBConnection getNewConnection(String driver,
-                                            String url,
-                                            String username,
-                                            String password)
+    protected DBConnection getNewConnection()
         throws Exception
     {
-        // PoolBrokerService keeps a collection of ConnectionPools,
-        // each one of which contains connections to a single database.
+        // PoolBrokerService keeps a collection of ConnectionPools,
+        // each one of which contains connections to a single database.
         // The initialization of a pool should only occur once.
         if (db == null)
         {
@@ -252,26 +349,20 @@
     }

     /**
-     * Helper function that attempts to pop a connection off the pool's
stack,
-     * handling the case where the popped connection has become invalid
by
+     * Helper function that attempts to pop a connection off the pool's
stack,
+     * handling the case where the popped connection has become invalid
by
      * creating a new connection.
      *
-     * @param driver The fully-qualified name of the JDBC driver to
use.
-     * @param url The URL of the database from which the connection is
-     * desired.
-     * @param username The name of the database user.
-     * @param password The password of the database user.
      * @return A database connection.
      * @exception Exception
      */
-    private DBConnection popConnection( String driver, String url,
-                                        String username, String
password )
+    private DBConnection popConnection()
         throws Exception
     {
         while ( !pool.empty() )
         {
             DBConnection con = (DBConnection) pool.pop();
-
+
             // It's really not safe to assume this connection is
             // valid even though it's checked before being pooled.
             if ( isValid(con) )
@@ -285,41 +376,33 @@
                 con.close();
                 totalConnections--;
                 connectionAttemptsCounter = 0;
-
-                // If the pool is now empty, create a new connection.
We're
-                // guaranteed not to exceed the connection limit since
we
-                // just killed off one or more invalid connections, and
no
+
+                // If the pool is now empty, create a new connection.
We're
+                // guaranteed not to exceed the connection limit since
we
+                // just killed off one or more invalid connections, and
no
                 // one else can be accessing this cache right now.
                 if ( pool.empty() )
                 {
-                    return getNewConnection(driver, url, username,
password);
+                    return getNewConnection();
                 }
             }
         }
-
-        // The connection pool was empty to start with--don't call this
+
+        // The connection pool was empty to start with--don't call this
         // routine if there's no connection to pop!
         // TODO: Propose general Turbine assertion failure exception?
-PGO
-        throw new Exception("Assertaion failure: Attempted to pop " +
+        throw new Exception("Assertaion failure: Attempted to pop " +
                             "connection from empty pool!");
     }
-
+
     /**
      * Gets a pooled database connection.
      *
-     * @param driver The fully-qualified name of the JDBC driver to
use.
-     * @param url The URL of the database from which the connection is
-     * desired.
-     * @param username The name of the database user.
-     * @param password The password of the database user.
      * @return A database connection.
      * @exception ConnectionWaitTimeoutException Wait time exceeded.
      * @exception Exception No pooled connections.
      */
-    private synchronized DBConnection getPooledConnection(String
driver,
-                                                          String url,
-                                                          String
username,
-                                                          String
password)
+    private synchronized DBConnection getPooledConnection()
         throws ConnectionWaitTimeoutException, Exception
     {
         DBConnection con = null;
@@ -329,7 +412,7 @@
             connectionAttemptsCounter++;

             // The connection pool is empty and we cannot allocate any
new
-            // connections.  Wait the prescribed amount of time and see
if
+            // connections.  Wait the prescribed amount of time and see
if
             // a connection is returned.
             try
             {
@@ -347,11 +430,11 @@
                 // someone returning a connection.
                 throw new ConnectionWaitTimeoutException(url);
             }
-            con = popConnection( driver, url, username, password );
+            con = popConnection();
         }
         else
         {
-            con = popConnection( driver, url, username, password );
+            con = popConnection();
         }

         return con;
@@ -397,17 +480,20 @@
      * @param connection The database connection to release.
      * @exception Exception Trouble releasing the connection.
      */
-    public synchronized void releaseConnection(DBConnection connection)
+    public synchronized void releaseConnection(DBConnection dbconn)
         throws Exception
     {
-        if ( isValid(connection) )
+               // DBConnections MUST be unlinked when returned to the
pool
+               dbconn.unlink(this);
+
+        if ( isValid(dbconn) )
         {
-            pool.push(connection);
+            pool.push(dbconn);
             notify();
         }
         else
         {
-            connection.close();
+            dbconn.close();
             totalConnections--;
         }
     }
@@ -445,7 +531,7 @@
             }

             // Check for database connectivity problems.
-            if ( connectionAttemptsCounter >= maxConnectionAttempts )
+            if ( dbconnAttemptsCounter >= maxConnectionAttempts )
             {
                 // Dump all connections.
                 try















diff -u -r1.5 PoolBrokerService.java
--- PoolBrokerService.java      2000/09/22 02:03:04     1.5
+++ PoolBrokerService.java      2000/09/25 16:46:22
@@ -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
+                // ... sync and look again to avoid race collisions.
+                pool = (ConnectionPool) pools.get(name);
+                if ( pool == null )
+                {
+                    // Still not there.  Create and add.
+                    pool = new
ConnectionPool(driver,url,username,password);
+                    pools.put( name , pool );
+                }
+            }
+        }
+    }
+
+
+    /**
+     * This method returns a pool with the
+     * specified name.  The pool must either have been registered
+     * with the {@link registerConnection() registerConnection()}
+     * methd, or be specified in the property file using the
+     * following syntax:
+     *
      * <pre>
      * database.[name].driver
      * database.[name].url
@@ -156,24 +198,66 @@
      * @return A DBConnection.
      * @exception Exception, a generic exception.
      */
+    public ConnectionPool getPool(String name) throws Exception
+    {
+               ConnectionPool pool = (ConnectionPool) pools.get(name);
+
+               if ( pool == null ) {
+                       // If the pool is not in the hashtable, we must
register it
+                       registerPool(   name,
+                                                      
TurbineResources.getString(
+                                                              
"database." + name + ".driver", "" ),
+                            TurbineResources.getString(
+                                                              
"database." + name + ".url", "" ),
+                            TurbineResources.getString(
+                                                              
"database." +  name + ".username", "" ),
+                            TurbineResources.getString(
+                                                              
"database." +  name + ".password", "" ) );
+                       pool = (ConnectionPool) pools.get(name);
+               }
+
+               return pool;
+    }
+
+    /**
+     * This method returns the default pool:
+     *
+     * @return A the default ConnectionPool.
+     * @exception Exception, a generic exception.
+     */
+    public ConnectionPool getPool() throws Exception
+    {
+               return getPool(DEFAULT);
+    }
+
+
+    /**
+     * This method returns a DBConnection from the pool with the
+     * specified name.  The pool must either have been registered
+     * with the {@link registerConnection() registerConnection()}
+     * methd, or be specified in the property file using the
+     * following syntax:
+     *
+     * <pre>
+     * database.[name].driver
+     * database.[name].url
+     * database.[name].username
+     * database.[name].password
+     * </pre>
+     *
+     * @param name A String.
+     * @return A DBConnection.
+     * @exception Exception, a generic exception.
+     */
     public DBConnection getConnection(String name) throws Exception
     {
-        return getConnection( TurbineResources.getString( "database." +
-                                                          name +
-                                                          ".driver", ""
),
-                              TurbineResources.getString( "database." +
-                                                          name +
-                                                          ".url", "" ),
-                              TurbineResources.getString( "database." +
-                                                          name +
-                                                          ".username",
"" ),
-                              TurbineResources.getString( "database." +
-                                                          name +
-                                                          ".password",
"" ) );
+               // The getPool method ensures the validity
+               // of the returned pool
+               return getPool(name).getConnection();
     }

     /**
-     * This method returns a connection using the default values
+     * This method returns a DBConnection using the default values
      * specified in a properties file.
      *
      * @return A DBConnection.
@@ -186,7 +270,7 @@
     }

     /**
-     * This method returns a connection using the given parameters.
+     * This method returns a DBConnecton using the given parameters.
      *
      * @param driver The fully-qualified name of the JDBC driver to
use.
      * @param url The URL of the database from which the connection is
@@ -194,6 +278,8 @@
      * @param username The name of the database user.
      * @param password The password of the database user.
      * @return A DBConnection.
+     * @deprecated Database parameters should not be specified each
+     * time a DBConnection is fetched from the service.
      * @exception Exception, a generic exception.
      */
      public DBConnection getConnection(String driver,
@@ -206,24 +292,17 @@
         url = url.trim();

         // Quick (non-sync) check for the pool we want.
-        pool = (ConnectionPool) pools.get(url);
+        // NOTE: Here we must not call getPool(), since the pool
+        // is almost certainly not defined in the properties file
+        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);
-                if ( pool == null )
-                {
-                    // Still not there.  Create and add.
-                    pool = new ConnectionPool();
-                    pools.put( url, pool );
-                }
-            }
+            registerPool(
+                               url + username, driver,  url, username,
password);
+               pool = (ConnectionPool) pools.get(url + username);
         }

-        return pool.getConnection(driver, url, username, password);
+        return pool.getConnection();
     }

     /**
@@ -328,17 +407,7 @@
     public DB getDB(String name)
         throws Exception
     {
-        String url = TurbineResources.getString( "database." +
-                                                 name +
-                                                 ".url", "" ).trim();
-        ConnectionPool cp = (ConnectionPool)pools.get(url);
-        if ( cp == null)
-        {
-            DBConnection dbcon = getConnection(name);
-            releaseConnection(dbcon);
-            cp = (ConnectionPool)pools.get(url);
-        }
-        return cp.getDB();
+        return getPool(name).getDB();
     }

     /**
@@ -364,30 +433,18 @@
     /**
      * Release a connection back to the database pool.
      *
-     * @param connection A DBConnection.
+     * @param dbconn A DBConnection.
      * @exception Exception, a generic exception.
      */
-    public void releaseConnection(DBConnection connection)
+    public void releaseConnection(DBConnection dbconn)
         throws Exception
     {
-        if ( connection != null )
+        if ( dbconn != null )
         {
-            String url = connection.getUrl();
-            if ( url != null )
-            {
-                ConnectionPool pool = null;
-                synchronized ( pools )
-                {
-                    pool = (ConnectionPool) pools.get( url );
-                }
-
-                // If there is a ConnecitonPool associated with this
-                // connection's URL, release the connection from that
pool.
-                if ( pool != null )
-                {
-                    pool.releaseConnection( connection );
-                }
-            }
+                       ConnectionPool pool = dbconn.getPool();
+                       if ( pools.containsValue( pool ) ) {
+                               pool.releaseConnection( dbconn );
+                       }
         }
     }


------------------------------------------------------------
To subscribe:        [EMAIL PROTECTED]
To unsubscribe:      [EMAIL PROTECTED]
Search: <http://www.mail-archive.com/turbine%40list.working-dogs.com/>
Problems?:           [EMAIL PROTECTED]

Reply via email to