Eliminate connection/statement/result set leaks in TableQuery (#10582)

Includes some refactoring to extract common parts of FreeformQuery and
TableQuery to AbstractTransactionalQuery.

svn changeset:25541/svn branch:6.8

Added missing Serializable for AbstractTransactionalQuery (#10582).
    
svn changeset:25559/svn branch:6.8

Conflicts:
	server/src/com/vaadin/data/util/sqlcontainer/query/FreeformQuery.java
	server/src/com/vaadin/data/util/sqlcontainer/query/TableQuery.java

Change-Id: I3d055e0f071739ac4536fddb0f49b8d6b8e6d07b

3 files changed, 292 insertions, 230 deletions

diff --git a/server/src/com/vaadin/data/util/sqlcontainer/query/AbstractTransactionalQuery.java b/server/src/com/vaadin/data/util/sqlcontainer/query/AbstractTransactionalQuery.java
new file mode 100644
index 0000000000..3264118732
--- /dev/null
+++ b/server/src/com/vaadin/data/util/sqlcontainer/query/AbstractTransactionalQuery.java
@@ -0,0 +1,185 @@
+/*
+ * Copyright 2000-2013 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.data.util.sqlcontainer.query;
+
+import java.io.Serializable;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+import com.vaadin.data.util.sqlcontainer.connection.JDBCConnectionPool;
+
+/**
+ * Common base class for database query classes that handle connections and
+ * transactions.
+ * 
+ * @author Vaadin Ltd
+ * @since 6.8.9
+ */
+abstract class AbstractTransactionalQuery implements Serializable {
+
+    private JDBCConnectionPool connectionPool;
+    private transient Connection activeConnection;
+
+    AbstractTransactionalQuery() {
+    }
+
+    AbstractTransactionalQuery(JDBCConnectionPool connectionPool) {
+        this.connectionPool = connectionPool;
+    }
+
+    /**
+     * Reserves a connection with auto-commit off if no transaction is in
+     * progress.
+     * 
+     * @throws IllegalStateException
+     *             if a transaction is already open
+     * @throws SQLException
+     *             if a connection could not be obtained or configured
+     */
+    public void beginTransaction() throws UnsupportedOperationException,
+            SQLException {
+        if (isInTransaction()) {
+            throw new IllegalStateException("A transaction is already active!");
+        }
+        activeConnection = connectionPool.reserveConnection();
+        activeConnection.setAutoCommit(false);
+    }
+
+    /**
+     * Commits (if not in auto-commit mode) and releases the active connection.
+     * 
+     * @throws SQLException
+     *             if not in a transaction managed by this query
+     */
+    public void commit() throws UnsupportedOperationException, SQLException {
+        if (!isInTransaction()) {
+            throw new SQLException("No active transaction");
+        }
+        if (!activeConnection.getAutoCommit()) {
+            activeConnection.commit();
+        }
+        connectionPool.releaseConnection(activeConnection);
+        activeConnection = null;
+    }
+
+    /**
+     * Rolls back and releases the active connection.
+     * 
+     * @throws SQLException
+     *             if not in a transaction managed by this query
+     */
+    public void rollback() throws UnsupportedOperationException, SQLException {
+        if (!isInTransaction()) {
+            throw new SQLException("No active transaction");
+        }
+        activeConnection.rollback();
+        connectionPool.releaseConnection(activeConnection);
+        activeConnection = null;
+    }
+
+    /**
+     * Check that a transaction is active.
+     * 
+     * @throws SQLException
+     *             if no active transaction
+     */
+    protected void ensureTransaction() throws SQLException {
+        if (!isInTransaction()) {
+            throw new SQLException("No active transaction!");
+        }
+    }
+
+    /**
+     * Closes a statement and a resultset, then releases the connection if it is
+     * not part of an active transaction. A failure in closing one of the
+     * parameters does not prevent closing the rest.
+     * 
+     * If the statement is a {@link PreparedStatement}, its parameters are
+     * cleared prior to closing the statement.
+     * 
+     * Although JDBC specification does state that closing a statement closes
+     * its result set and closing a connection closes statements and result
+     * sets, this method does try to close the result set and statement
+     * explicitly whenever not null. This can guard against bugs in certain JDBC
+     * drivers and reduce leaks in case e.g. closing the result set succeeds but
+     * closing the statement or connection fails.
+     * 
+     * @param conn
+     *            the connection to release
+     * @param statement
+     *            the statement to close, may be null to skip closing
+     * @param rs
+     *            the result set to close, may be null to skip closing
+     * @throws SQLException
+     *             if closing the result set or the statement fails
+     */
+    protected void releaseConnection(Connection conn, Statement statement,
+            ResultSet rs) throws SQLException {
+        try {
+            try {
+                if (null != rs) {
+                    rs.close();
+                }
+            } finally {
+                if (null != statement) {
+                    if (statement instanceof PreparedStatement) {
+                        try {
+                            ((PreparedStatement) statement).clearParameters();
+                        } catch (Exception e) {
+                            // will be closed below anyway
+                        }
+                    }
+                    statement.close();
+                }
+            }
+        } finally {
+            releaseConnection(conn);
+        }
+    }
+
+    /**
+     * Returns the currently active connection, reserves and returns a new
+     * connection if no active connection.
+     * 
+     * @return previously active or newly reserved connection
+     * @throws SQLException
+     */
+    protected Connection getConnection() throws SQLException {
+        if (activeConnection != null) {
+            return activeConnection;
+        }
+        return connectionPool.reserveConnection();
+    }
+
+    protected boolean isInTransaction() {
+        return activeConnection != null;
+    }
+
+    /**
+     * Releases the connection if it is not part of an active transaction.
+     * 
+     * @param conn
+     *            the connection to release
+     */
+    private void releaseConnection(Connection conn) {
+        if (conn != activeConnection && conn != null) {
+            connectionPool.releaseConnection(conn);
+        }
+    }
+}
diff --git a/server/src/com/vaadin/data/util/sqlcontainer/query/FreeformQuery.java b/server/src/com/vaadin/data/util/sqlcontainer/query/FreeformQuery.java
index 299183f5e6..6895e02147 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/query/FreeformQuery.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/query/FreeformQuery.java
@@ -34,13 +34,12 @@ import com.vaadin.data.util.sqlcontainer.query.generator.StatementHelper;
 import com.vaadin.data.util.sqlcontainer.query.generator.filter.QueryBuilder;
 
 @SuppressWarnings("serial")
-public class FreeformQuery implements QueryDelegate {
+public class FreeformQuery extends AbstractTransactionalQuery implements
+        QueryDelegate {
 
     FreeformQueryDelegate delegate = null;
     private String queryString;
     private List<String> primaryKeyColumns;
-    private JDBCConnectionPool connectionPool;
-    private transient Connection activeConnection = null;
 
     /**
      * Prevent no-parameters instantiation of FreeformQuery
@@ -67,6 +66,7 @@ public class FreeformQuery implements QueryDelegate {
     @Deprecated
     public FreeformQuery(String queryString, List<String> primaryKeyColumns,
             JDBCConnectionPool connectionPool) {
+        super(connectionPool);
         if (primaryKeyColumns == null) {
             primaryKeyColumns = new ArrayList<String>();
         }
@@ -83,7 +83,6 @@ public class FreeformQuery implements QueryDelegate {
         this.queryString = queryString;
         this.primaryKeyColumns = Collections
                 .unmodifiableList(primaryKeyColumns);
-        this.connectionPool = connectionPool;
     }
 
     /**
@@ -189,13 +188,6 @@ public class FreeformQuery implements QueryDelegate {
         return count;
     }
 
-    private Connection getConnection() throws SQLException {
-        if (activeConnection != null) {
-            return activeConnection;
-        }
-        return connectionPool.reserveConnection();
-    }
-
     /**
      * Fetches the results for the query. This implementation always fetches the
      * entire record set, ignoring the offset and page length parameters. In
@@ -210,9 +202,7 @@ public class FreeformQuery implements QueryDelegate {
     @Override
     @SuppressWarnings({ "deprecation", "finally" })
     public ResultSet getResults(int offset, int pagelength) throws SQLException {
-        if (activeConnection == null) {
-            throw new SQLException("No active transaction!");
-        }
+        ensureTransaction();
         String query = queryString;
         if (delegate != null) {
             /* First try using prepared statement */
@@ -220,8 +210,8 @@ public class FreeformQuery implements QueryDelegate {
                 try {
                     StatementHelper sh = ((FreeformStatementDelegate) delegate)
                             .getQueryStatement(offset, pagelength);
-                    PreparedStatement pstmt = activeConnection
-                            .prepareStatement(sh.getQueryString());
+                    PreparedStatement pstmt = getConnection().prepareStatement(
+                            sh.getQueryString());
                     sh.setParameterValuesToStatement(pstmt);
                     return pstmt.executeQuery();
                 } catch (UnsupportedOperationException e) {
@@ -234,7 +224,7 @@ public class FreeformQuery implements QueryDelegate {
                 // This is fine, we'll just use the default queryString.
             }
         }
-        Statement statement = activeConnection.createStatement();
+        Statement statement = getConnection().createStatement();
         ResultSet rs;
         try {
             rs = statement.executeQuery(query);
@@ -322,14 +312,14 @@ public class FreeformQuery implements QueryDelegate {
      */
     @Override
     public int storeRow(RowItem row) throws SQLException {
-        if (activeConnection == null) {
+        if (!isInTransaction()) {
             throw new IllegalStateException("No transaction is active!");
         } else if (primaryKeyColumns.isEmpty()) {
             throw new UnsupportedOperationException(
                     "Cannot store items fetched with a read-only freeform query!");
         }
         if (delegate != null) {
-            return delegate.storeRow(activeConnection, row);
+            return delegate.storeRow(getConnection(), row);
         } else {
             throw new UnsupportedOperationException(
                     "FreeFormQueryDelegate not set!");
@@ -345,68 +335,36 @@ public class FreeformQuery implements QueryDelegate {
      */
     @Override
     public boolean removeRow(RowItem row) throws SQLException {
-        if (activeConnection == null) {
+        if (!isInTransaction()) {
             throw new IllegalStateException("No transaction is active!");
         } else if (primaryKeyColumns.isEmpty()) {
             throw new UnsupportedOperationException(
                     "Cannot remove items fetched with a read-only freeform query!");
         }
         if (delegate != null) {
-            return delegate.removeRow(activeConnection, row);
+            return delegate.removeRow(getConnection(), row);
         } else {
             throw new UnsupportedOperationException(
                     "FreeFormQueryDelegate not set!");
         }
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see
-     * com.vaadin.data.util.sqlcontainer.query.QueryDelegate#beginTransaction()
-     */
     @Override
     public synchronized void beginTransaction()
             throws UnsupportedOperationException, SQLException {
-        if (activeConnection != null) {
-            throw new IllegalStateException("A transaction is already active!");
-        }
-        activeConnection = connectionPool.reserveConnection();
-        activeConnection.setAutoCommit(false);
+        super.beginTransaction();
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see com.vaadin.data.util.sqlcontainer.query.QueryDelegate#commit()
-     */
     @Override
     public synchronized void commit() throws UnsupportedOperationException,
             SQLException {
-        if (activeConnection == null) {
-            throw new SQLException("No active transaction");
-        }
-        if (!activeConnection.getAutoCommit()) {
-            activeConnection.commit();
-        }
-        connectionPool.releaseConnection(activeConnection);
-        activeConnection = null;
+        super.commit();
     }
 
-    /*
-     * (non-Javadoc)
-     * 
-     * @see com.vaadin.data.util.sqlcontainer.query.QueryDelegate#rollback()
-     */
     @Override
     public synchronized void rollback() throws UnsupportedOperationException,
             SQLException {
-        if (activeConnection == null) {
-            throw new SQLException("No active transaction");
-        }
-        activeConnection.rollback();
-        connectionPool.releaseConnection(activeConnection);
-        activeConnection = null;
+        super.rollback();
     }
 
     /*
@@ -492,66 +450,6 @@ public class FreeformQuery implements QueryDelegate {
         return contains;
     }
 
-    /**
-     * Releases the connection if it is not part of an active transaction.
-     * 
-     * @param conn
-     *            the connection to release
-     */
-    private void releaseConnection(Connection conn) {
-        if (conn != activeConnection) {
-            connectionPool.releaseConnection(conn);
-        }
-    }
-
-    /**
-     * Closes a statement and a resultset, then releases the connection if it is
-     * not part of an active transaction. A failure in closing one of the
-     * parameters does not prevent closing the rest.
-     * 
-     * If the statement is a {@link PreparedStatement}, its parameters are
-     * cleared prior to closing the statement.
-     * 
-     * Although JDBC specification does state that closing a statement closes
-     * its result set and closing a connection closes statements and result
-     * sets, this method does try to close the result set and statement
-     * explicitly whenever not null. This can guard against bugs in certain JDBC
-     * drivers and reduce leaks in case e.g. closing the result set succeeds but
-     * closing the statement or connection fails.
-     * 
-     * @param conn
-     *            the connection to release
-     * @param statement
-     *            the statement to close, may be null to skip closing
-     * @param rs
-     *            the result set to close, may be null to skip closing
-     * @throws SQLException
-     *             if closing the result set or the statement fails
-     */
-    private void releaseConnection(Connection conn, Statement statement,
-            ResultSet rs) throws SQLException {
-        try {
-            try {
-                if (null != rs) {
-                    rs.close();
-                }
-            } finally {
-                if (null != statement) {
-                    if (statement instanceof PreparedStatement) {
-                        try {
-                            ((PreparedStatement) statement).clearParameters();
-                        } catch (Exception e) {
-                            // will be closed below anyway
-                        }
-                    }
-                    statement.close();
-                }
-            }
-        } finally {
-            releaseConnection(conn);
-        }
-    }
-
     private String modifyWhereClause(Object... keys) {
         // Build the where rules for the provided keys
         StringBuffer where = new StringBuffer();
diff --git a/server/src/com/vaadin/data/util/sqlcontainer/query/TableQuery.java b/server/src/com/vaadin/data/util/sqlcontainer/query/TableQuery.java
index 63e4b3362c..caed5526e3 100644
--- a/server/src/com/vaadin/data/util/sqlcontainer/query/TableQuery.java
+++ b/server/src/com/vaadin/data/util/sqlcontainer/query/TableQuery.java
@@ -47,8 +47,8 @@ import com.vaadin.data.util.sqlcontainer.query.generator.SQLGenerator;
 import com.vaadin.data.util.sqlcontainer.query.generator.StatementHelper;
 
 @SuppressWarnings("serial")
-public class TableQuery implements QueryDelegate,
-        QueryDelegate.RowIdChangeNotifier {
+public class TableQuery extends AbstractTransactionalQuery implements
+        QueryDelegate, QueryDelegate.RowIdChangeNotifier {
 
     /** Table name, primary key column name(s) and version column name */
     private String tableName;
@@ -62,18 +62,13 @@ public class TableQuery implements QueryDelegate,
     /** SQLGenerator instance to use for generating queries */
     private SQLGenerator sqlGenerator;
 
-    /** Fields related to Connection and Transaction handling */
-    private JDBCConnectionPool connectionPool;
-    private transient Connection activeConnection;
-    private boolean transactionOpen;
-
     /** Row ID change listeners */
     private LinkedList<RowIdChangeListener> rowIdChangeListeners;
     /** Row ID change events, stored until commit() is called */
     private final List<RowIdChangeEvent> bufferedEvents = new ArrayList<RowIdChangeEvent>();
 
     /** Set to true to output generated SQL Queries to System.out */
-    private boolean debug = false;
+    private final boolean debug = false;
 
     /** Prevent no-parameters instantiation of TableQuery */
     @SuppressWarnings("unused")
@@ -93,6 +88,7 @@ public class TableQuery implements QueryDelegate,
      */
     public TableQuery(String tableName, JDBCConnectionPool connectionPool,
             SQLGenerator sqlGenerator) {
+        super(connectionPool);
         if (tableName == null || tableName.trim().length() < 1
                 || connectionPool == null || sqlGenerator == null) {
             throw new IllegalArgumentException(
@@ -100,7 +96,6 @@ public class TableQuery implements QueryDelegate,
         }
         this.tableName = tableName;
         this.sqlGenerator = sqlGenerator;
-        this.connectionPool = connectionPool;
         fetchMetaData();
     }
 
@@ -129,17 +124,27 @@ public class TableQuery implements QueryDelegate,
         StatementHelper sh = sqlGenerator.generateSelectQuery(tableName,
                 filters, null, 0, 0, "COUNT(*)");
         boolean shouldCloseTransaction = false;
-        if (!transactionOpen) {
+        if (!isInTransaction()) {
             shouldCloseTransaction = true;
             beginTransaction();
         }
-        ResultSet r = executeQuery(sh);
-        r.next();
-        int count = r.getInt(1);
-        r.getStatement().close();
-        r.close();
-        if (shouldCloseTransaction) {
-            commit();
+        ResultSet r = null;
+        int count = -1;
+        try {
+            r = executeQuery(sh);
+            r.next();
+            count = r.getInt(1);
+        } finally {
+            try {
+                if (r != null) {
+                    releaseConnection(r.getStatement().getConnection(),
+                            r.getStatement(), r);
+                }
+            } finally {
+                if (shouldCloseTransaction) {
+                    commit();
+                }
+            }
         }
         return count;
     }
@@ -240,28 +245,30 @@ public class TableQuery implements QueryDelegate,
         setVersionColumnFlagInProperty(row);
         /* Generate query */
         StatementHelper sh = sqlGenerator.generateInsertQuery(tableName, row);
-        PreparedStatement pstmt = activeConnection.prepareStatement(
-                sh.getQueryString(), primaryKeyColumns.toArray(new String[0]));
-        sh.setParameterValuesToStatement(pstmt);
-        getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
-        int result = pstmt.executeUpdate();
-        if (result > 0) {
-            /*
-             * If affected rows exist, we'll get the new RowId, commit the
-             * transaction and return the new RowId.
-             */
-            ResultSet generatedKeys = pstmt.getGeneratedKeys();
-            RowId newId = getNewRowId(row, generatedKeys);
-            generatedKeys.close();
-            pstmt.clearParameters();
-            pstmt.close();
+        Connection connection = null;
+        PreparedStatement pstmt = null;
+        ResultSet generatedKeys = null;
+        connection = getConnection();
+        try {
+            pstmt = connection.prepareStatement(sh.getQueryString(),
+                    primaryKeyColumns.toArray(new String[0]));
+            sh.setParameterValuesToStatement(pstmt);
+            getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
+            int result = pstmt.executeUpdate();
+            RowId newId = null;
+            if (result > 0) {
+                /*
+                 * If affected rows exist, we'll get the new RowId, commit the
+                 * transaction and return the new RowId.
+                 */
+                generatedKeys = pstmt.getGeneratedKeys();
+                newId = getNewRowId(row, generatedKeys);
+            }
+            // transaction has to be closed in any case
             commit();
             return newId;
-        } else {
-            pstmt.clearParameters();
-            pstmt.close();
-            /* On failure return null */
-            return null;
+        } finally {
+            releaseConnection(connection, pstmt, generatedKeys);
         }
     }
 
@@ -307,14 +314,8 @@ public class TableQuery implements QueryDelegate,
     @Override
     public void beginTransaction() throws UnsupportedOperationException,
             SQLException {
-        if (transactionOpen && activeConnection != null) {
-            throw new IllegalStateException();
-        }
-
         getLogger().log(Level.FINE, "DB -> begin transaction");
-        activeConnection = connectionPool.reserveConnection();
-        activeConnection.setAutoCommit(false);
-        transactionOpen = true;
+        super.beginTransaction();
     }
 
     /*
@@ -324,14 +325,8 @@ public class TableQuery implements QueryDelegate,
      */
     @Override
     public void commit() throws UnsupportedOperationException, SQLException {
-        if (transactionOpen && activeConnection != null) {
-            getLogger().log(Level.FINE, "DB -> commit");
-            activeConnection.commit();
-            connectionPool.releaseConnection(activeConnection);
-        } else {
-            throw new SQLException("No active transaction");
-        }
-        transactionOpen = false;
+        getLogger().log(Level.FINE, "DB -> commit");
+        super.commit();
 
         /* Handle firing row ID change events */
         RowIdChangeEvent[] unFiredEvents = bufferedEvents
@@ -353,14 +348,8 @@ public class TableQuery implements QueryDelegate,
      */
     @Override
     public void rollback() throws UnsupportedOperationException, SQLException {
-        if (transactionOpen && activeConnection != null) {
-            getLogger().log(Level.FINE, "DB -> rollback");
-            activeConnection.rollback();
-            connectionPool.releaseConnection(activeConnection);
-        } else {
-            throw new SQLException("No active transaction");
-        }
-        transactionOpen = false;
+        getLogger().log(Level.FINE, "DB -> rollback");
+        super.rollback();
     }
 
     /*
@@ -402,16 +391,18 @@ public class TableQuery implements QueryDelegate,
      * @throws SQLException
      */
     private ResultSet executeQuery(StatementHelper sh) throws SQLException {
-        Connection c = null;
-        if (transactionOpen && activeConnection != null) {
-            c = activeConnection;
-        } else {
-            throw new SQLException("No active transaction!");
+        ensureTransaction();
+        Connection connection = getConnection();
+        PreparedStatement pstmt = null;
+        try {
+            pstmt = connection.prepareStatement(sh.getQueryString());
+            sh.setParameterValuesToStatement(pstmt);
+            getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
+            return pstmt.executeQuery();
+        } catch (SQLException e) {
+            releaseConnection(null, pstmt, null);
+            throw e;
         }
-        PreparedStatement pstmt = c.prepareStatement(sh.getQueryString());
-        sh.setParameterValuesToStatement(pstmt);
-        getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
-        return pstmt.executeQuery();
     }
 
     /**
@@ -426,27 +417,17 @@ public class TableQuery implements QueryDelegate,
      * @throws SQLException
      */
     private int executeUpdate(StatementHelper sh) throws SQLException {
-        Connection c = null;
         PreparedStatement pstmt = null;
+        Connection connection = null;
         try {
-            if (transactionOpen && activeConnection != null) {
-                c = activeConnection;
-            } else {
-                c = connectionPool.reserveConnection();
-            }
-            pstmt = c.prepareStatement(sh.getQueryString());
+            connection = getConnection();
+            pstmt = connection.prepareStatement(sh.getQueryString());
             sh.setParameterValuesToStatement(pstmt);
             getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
             int retval = pstmt.executeUpdate();
             return retval;
         } finally {
-            if (pstmt != null) {
-                pstmt.clearParameters();
-                pstmt.close();
-            }
-            if (!transactionOpen) {
-                connectionPool.releaseConnection(c);
-            }
+            releaseConnection(connection, pstmt, null);
         }
     }
 
@@ -467,16 +448,12 @@ public class TableQuery implements QueryDelegate,
      */
     private int executeUpdateReturnKeys(StatementHelper sh, RowItem row)
             throws SQLException {
-        Connection c = null;
         PreparedStatement pstmt = null;
         ResultSet genKeys = null;
+        Connection connection = null;
         try {
-            if (transactionOpen && activeConnection != null) {
-                c = activeConnection;
-            } else {
-                c = connectionPool.reserveConnection();
-            }
-            pstmt = c.prepareStatement(sh.getQueryString(),
+            connection = getConnection();
+            pstmt = connection.prepareStatement(sh.getQueryString(),
                     primaryKeyColumns.toArray(new String[0]));
             sh.setParameterValuesToStatement(pstmt);
             getLogger().log(Level.FINE, "DB -> {0}", sh.getQueryString());
@@ -486,16 +463,7 @@ public class TableQuery implements QueryDelegate,
             bufferedEvents.add(new RowIdChangeEvent(row.getId(), newId));
             return result;
         } finally {
-            if (genKeys != null) {
-                genKeys.close();
-            }
-            if (pstmt != null) {
-                pstmt.clearParameters();
-                pstmt.close();
-            }
-            if (!transactionOpen) {
-                connectionPool.releaseConnection(c);
-            }
+            releaseConnection(connection, pstmt, genKeys);
         }
     }
 
@@ -505,13 +473,15 @@ public class TableQuery implements QueryDelegate,
      * Also tries to get the escape string to be used in search strings.
      */
     private void fetchMetaData() {
-        Connection c = null;
+        Connection connection = null;
+        ResultSet rs = null;
+        ResultSet tables = null;
         try {
-            c = connectionPool.reserveConnection();
-            DatabaseMetaData dbmd = c.getMetaData();
+            connection = getConnection();
+            DatabaseMetaData dbmd = connection.getMetaData();
             if (dbmd != null) {
                 tableName = SQLUtil.escapeSQL(tableName);
-                ResultSet tables = dbmd.getTables(null, null, tableName, null);
+                tables = dbmd.getTables(null, null, tableName, null);
                 if (!tables.next()) {
                     tables = dbmd.getTables(null, null,
                             tableName.toUpperCase(), null);
@@ -525,7 +495,7 @@ public class TableQuery implements QueryDelegate,
                     }
                 }
                 tables.close();
-                ResultSet rs = dbmd.getPrimaryKeys(null, null, tableName);
+                rs = dbmd.getPrimaryKeys(null, null, tableName);
                 List<String> names = new ArrayList<String>();
                 while (rs.next()) {
                     names.add(rs.getString("COLUMN_NAME"));
@@ -554,7 +524,15 @@ public class TableQuery implements QueryDelegate,
         } catch (SQLException e) {
             throw new RuntimeException(e);
         } finally {
-            connectionPool.releaseConnection(c);
+            try {
+                releaseConnection(connection, null, rs);
+            } catch (SQLException ignore) {
+            } finally {
+                try {
+                    tables.close();
+                } catch (SQLException ignore) {
+                }
+            }
         }
     }
 
@@ -648,7 +626,7 @@ public class TableQuery implements QueryDelegate,
                 filtersAndKeys, orderBys, 0, 0, "*");
 
         boolean shouldCloseTransaction = false;
-        if (!transactionOpen) {
+        if (!isInTransaction()) {
             shouldCloseTransaction = true;
             beginTransaction();
         }
@@ -658,14 +636,15 @@ public class TableQuery implements QueryDelegate,
             boolean contains = rs.next();
             return contains;
         } finally {
-            if (rs != null) {
-                if (rs.getStatement() != null) {
-                    rs.getStatement().close();
+            try {
+                if (rs != null) {
+                    releaseConnection(rs.getStatement().getConnection(),
+                            rs.getStatement(), rs);
+                }
+            } finally {
+                if (shouldCloseTransaction) {
+                    commit();
                 }
-                rs.close();
-            }
-            if (shouldCloseTransaction) {
-                commit();
             }
         }
     }