Changeset 3541

Timestamp:

02/10/10 11:50:31 (6 months ago)

Author:

Maverick

Message:

Bugfixes for connection leaking. This was caused by a HashMap<String, Connection> where the surrounding code was trying to store multiple Connections on the same key (which only overwrites the previous Connection with the same key).

Files:

• trunk/twcore/src/twcore/core/sql/SQLConnectionPool.java (modified) (15 diffs)

trunk/twcore/src/twcore/core/sql/SQLConnectionPool.java

@@ -10 +10 @@
 import java.util.Map;
 import java.util.Vector;
+
+import twcore.core.util.Tools;
 
 /**
@@ -32 +34 @@
                                                 //         connection is available
     private Vector<Connection>  availableConnections;       // Connections not in use
-    private Map<String, Connection>  busyConnections;            // Connections currently querying
+    private Map<String, Vector<Connection>>  busyConnections;            // Connections currently querying
 
     private boolean connectionPending = false;  // True if a connection is being made
@@ -67 +69 @@
         }
         availableConnections = new Vector<Connection>(initialConnections);
-        busyConnections = Collections.synchronizedMap(new HashMap<String, Connection>());
+        busyConnections = Collections.synchronizedMap(new HashMap<String, Vector<Connection>>());
         for(int i=0; i<initialConnections; i++) {
             availableConnections.addElement(makeNewConnection());
@@ -86 +88 @@
         try{
             Statement stmt = conn.createStatement();
+            
             stmt.execute( query );
-            free( conn );
+            
             ResultSet set = stmt.getResultSet();
             // If ResultSet is null (INSERT statement), get auto-generated ID if available
@@ -94 +97 @@
             else
                 return stmt.getGeneratedKeys();
-        }catch( SQLException e ){
-            free( conn );
-
-            //throw e;
-
-            // DEBUG: Returning null to generate NullPointerExceptions in bots that
-            // do not catch SQLExceptions
-            return null;
+        } catch( SQLException e ){
+                throw e;
+                
+        } finally {
+            free( DEFAULT_UNIQUE_ID, conn );
         }
     }
@@ -121 +121 @@
      */
     public synchronized Connection getConnection(String uniqueID) throws SQLException {
-        if(uniqueID.equals("0")==false) {
+        if(uniqueID.equals(DEFAULT_UNIQUE_ID)==false) {
             if(busyConnections.containsKey(uniqueID)) {
-                return busyConnections.get(uniqueID);
+                return busyConnections.get(uniqueID).firstElement();
             }
         }
         
-        if (availableConnections.isEmpty()==false) {
-            Connection existingConnection = availableConnections.lastElement();
+        if (availableConnections.isEmpty()==false) { // if there are availableConnections
+                
+                // Get an connection from list of available connections and remove it from the list
+                Connection availableConnection = availableConnections.lastElement();
             int lastIndex = availableConnections.size() - 1;
             availableConnections.removeElementAt(lastIndex);
-            if (existingConnection.isClosed()) {
+            
+            if (availableConnection.isClosed()) {
+                
+                // if the connection is closed, try again by calling this method again - the availableConnection is lost
                 notifyAll(); // Freed up a spot for anybody waiting
                 return(getConnection());
+                
             } else {
-                busyConnections.put(uniqueID, existingConnection);
-                return(existingConnection);
-            }
-        } else {
+                if(!busyConnections.containsKey(uniqueID)) {
+                        busyConnections.put(uniqueID, new Vector<Connection>());
+                }
+                busyConnections.get(uniqueID).add(availableConnection);
+                return availableConnection;
+            }
+            
+        } else {        // there are no available connections
+                
+                // Check if we reached the maximum amount of connections
             if(( totalConnections() < maxConnections ) && !connectionPending ) {
                 makeBackgroundConnection();
@@ -144 +156 @@
                 throw new SQLException("Connection limit reached");
             }
+            
             try {
                 wait();
             } catch(InterruptedException ie) {}
-            // Someone freed up a connection, so try again.
+            // Someone freed up a connection or a new connection has been made, try again.
+            
             return getConnection(uniqueID);
         }
@@ -188 +202 @@
         try {
             Connection connection = makeNewConnection();
+            
             synchronized(this) {
                 availableConnections.addElement(connection);
@@ -193 +208 @@
                 notifyAll();
             }
-        } catch(Exception e) {
+        } catch(SQLException e) {
+                Tools.printStackTrace(e);
         }
     }
@@ -216 +232 @@
 
     /**
-     * Free a connection back to the available connection pool.  This will notify
+     * Free a specific connection back to the available connection pool.  This will notify
      * other threads that are waiting (using wait()) for a connection in getConnection()
      * that a new connection is now available.
@@ -222 +238 @@
      */
     public synchronized void free( String uniqueID, Connection connection ) {
-        if(busyConnections.containsKey(uniqueID)) {
-            busyConnections.remove(uniqueID);
+        if(busyConnections.containsKey(uniqueID) && busyConnections.get(uniqueID).contains(connection)) {
+                
+                // Remove connection from the vector
+                busyConnections.get(uniqueID).remove(connection);
+                
+                // Remove HashMap entry if vector has become empty
+                if(busyConnections.get(uniqueID).isEmpty()) {
+                        busyConnections.remove(uniqueID);
+                }
+                
             availableConnections.addElement(connection);
+            
             // Wake up threads that are waiting for a connection
             notifyAll();
@@ -230 +255 @@
     }
     
-    public synchronized void free( Connection connection ) {
-        this.free( DEFAULT_UNIQUE_ID, connection);
-    }
-
     /**
      * @return Total number of connections either available or currently busy
      */
     public synchronized int totalConnections() {
-        return(availableConnections.size() +
-        busyConnections.size());
+        return(availableConnections.size() + busyConnections.size());
     }
 
@@ -249 +269 @@
         availableConnections = new Vector<Connection>();
         closeBusyConnections(busyConnections);
-        busyConnections = new HashMap<String,Connection>();
+        busyConnections = new HashMap<String,Vector<Connection>>();
     }
 
@@ -271 +291 @@
      * @param connections Vector containing connections to close
      */
-    private void closeBusyConnections(Map<String,Connection> connections) {
-        try {
-            for(Connection conn:connections.values()) {
-                if (!conn.isClosed()) {
-                    conn.close();
-                }
-            }
+    private void closeBusyConnections(Map<String,Vector<Connection>> connections) {
+        try {
+                for(Vector<Connection> conns:connections.values()) {
+                    for(Connection conn:conns) {
+                        if (!conn.isClosed()) {
+                            conn.close();
+                        }
+                    }
+                }
         } catch(SQLException sqle) {
         }
@@ -293 +315 @@
      */
     public synchronized String toString() {
-        String info =
+        /*String info =
         "SQL pool " + poolName + ": " + totalConnections() + "/"
         + maxConnections + " connections online, " + busyConnections.size()
-        + " in use";
-        return(info);
+        + " in use";*/
+        return "SQL pool "+ poolName + ": " + busyConnections.size() + "/" + availableConnections.size() + " connections online (background: "+getNumBackground()+") (max: "+maxConnections+")";
     }