db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r675870 - in /db/derby/code/trunk/java: client/org/apache/derby/client/am/ client/org/apache/derby/client/net/ engine/org/apache/derby/iapi/jdbc/ engine/org/apache/derby/impl/jdbc/ engine/org/apache/derby/jdbc/ testing/org/apache/derbyTesti...
Date Fri, 11 Jul 2008 08:55:31 GMT
Author: kahatlen
Date: Fri Jul 11 01:55:29 2008
New Revision: 675870

URL: http://svn.apache.org/viewvc?rev=675870&view=rev
Log:
DERBY-3319: Logical connections do not check if a transaction is active on close

Make sure that connections created by the different kinds of data
sources throw an exception on close if they are active. This is done
to get the same behaviour as with connections returned by
DriverManager.

Don't throw exception for connections with auto-commit on (since
they'll auto-commit the transaction as part of the close, and
therefore won't leave uncommitted operations around) or connections
that are part of an XA transaction (since those transactions can still
be committed/aborted via the associated XAResource after the closing
of the connection).

Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java
    db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnectionControl.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
    db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
    db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/J2EEDataSourceTest.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/Connection.java Fri Jul 11
01:55:29 2008
@@ -705,16 +705,30 @@
         closeX();
     }
 
+    /**
+     * Check if the transaction is in progress and the connection cannot be
+     * closed.
+     *
+     * @throws SqlException if the connection cannot be closed because the
+     * transaction is active
+     */
     void checkForTransactionInProgress() throws SqlException {
         // The following precondition matches CLI semantics, see SQLDisconnect()
-        if (transactionInProgress() && !allowCloseInUOW_()) {
+        if (transactionInProgress()) {
             throw new SqlException(agent_.logWriter_,
                     new ClientMessageId (SQLState.CANNOT_CLOSE_ACTIVE_CONNECTION));     
             
         }
     }
-    
+
+    /**
+     * Check if there are uncommitted operations in the current transaction
+     * that prevent us from closing the connection.
+     *
+     * @return {@code true} if the connection cannot be closed due to
+     * uncommitted operations in the transaction
+     */
     public boolean transactionInProgress() {
-        return !autoCommit_ && inUnitOfWork_;
+        return inUnitOfWork_ && !allowCloseInUOW_();
     }
 
     // This is a no-op if the connection is already closed.

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/am/LogicalConnection.java Fri
Jul 11 01:55:29 2008
@@ -84,6 +84,7 @@
                     new ClientMessageId(
                         SQLState.PHYSICAL_CONNECTION_ALREADY_CLOSED)));
             } else {
+                physicalConnection_.checkForTransactionInProgress();
                 physicalConnection_.closeForReuse(
                         pooledConnection_.isStatementPoolingEnabled());
                 if (!physicalConnection_.isGlobalPending_()) {

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/net/NetConnection.java Fri Jul
11 01:55:29 2008
@@ -1505,13 +1505,33 @@
         return closeStatementsOnClose;
     }
 
+    /**
+     * Check if the connection can be closed when there are uncommitted
+     * operations.
+     *
+     * @return if this connection can be closed when it has uncommitted
+     * operations, {@code true}; otherwise, {@code false}
+     */
     protected boolean allowCloseInUOW_() {
-        return false;
+        // We allow closing in unit of work in two cases:
+        //
+        //   1) if auto-commit is on, since then Connection.close() will cause
+        //   a commit so we won't leave uncommitted changes around
+        //
+        //   2) if we're not allowed to commit or roll back the transaction via
+        //   the connection object (because the it is part of an XA
+        //   transaction). In that case, commit and rollback are performed via
+        //   the XAResource, and it is therefore safe to close the connection.
+        //
+        // Otherwise, the transaction must be idle before a call to close() is
+        // allowed.
+
+        return autoCommit_ || !allowLocalCommitRollback_();
     }
 
     // Driver-specific determination if local COMMIT/ROLLBACK is allowed;
     // Allow local COMMIT/ROLLBACK only if we are not in an XA transaction
-    protected boolean allowLocalCommitRollback_() throws org.apache.derby.client.am.SqlException
{
+    protected boolean allowLocalCommitRollback_() {
        
     	if (getXAState() == XA_T0_NOT_ASSOCIATED) {
             return true;

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnection.java Fri
Jul 11 01:55:29 2008
@@ -166,6 +166,8 @@
 			return;
 
 		try {
+            control.checkClose();
+
 			if (!control.closingConnection()) {
 				isClosed = true;
 				return;

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnectionControl.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnectionControl.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnectionControl.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/jdbc/BrokeredConnectionControl.java
Fri Jul 11 01:55:29 2008
@@ -63,6 +63,14 @@
 	*/
 	public void checkCommit() throws SQLException;
 
+    /**
+     * Check if the brokered connection can be closed.
+     *
+     * @throws SQLException if it is not allowed to call close on the brokered
+     * connection
+     */
+    public void checkClose() throws SQLException;
+
 	/**
 		Can cursors be held across commits.
         @param downgrade true to downgrade the holdability,

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/jdbc/EmbedConnection.java Fri Jul
11 01:55:29 2008
@@ -1778,17 +1778,26 @@
      * @exception SQLException if a database-access error occurs.
      */
     public void close() throws SQLException {
-		// JDK 1.4 javadoc indicates close on a closed connection is a no-op
-		if (!isClosed() &&
-				(rootConnection == this) && 
-				(!autoCommit && !transactionIsIdle())) {
-			throw newSQLException(
-				SQLState.CANNOT_CLOSE_ACTIVE_CONNECTION);
-		}
-		
+        checkForTransactionInProgress();
 		close(exceptionClose);
 	}
 
+    /**
+     * Check if the transaction is active so that we cannot close down the
+     * connection. If auto-commit is on, the transaction is committed when the
+     * connection is closed, so it is always OK to close the connection in that
+     * case. Otherwise, throw an exception if a transaction is in progress.
+     *
+     * @throws SQLException if this transaction is active and the connection
+     * cannot be closed
+     */
+    public void checkForTransactionInProgress() throws SQLException {
+        if (!isClosed() && (rootConnection == this) &&
+                !autoCommit && !transactionIsIdle()) {
+            throw newSQLException(SQLState.CANNOT_CLOSE_ACTIVE_CONNECTION);
+        }
+    }
+
 	// This inner close takes the exception and calls 
 	// the context manager to make the connection close.
 	// The exception must be a session severity exception.

Modified: db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection.java Fri Jul
11 01:55:29 2008
@@ -153,11 +153,6 @@
 	{
 		checkActive();
 
-		// need to do this in case the connection is forcibly removed without
-		// first being closed.
-		closeCurrentConnectionHandle();
-
-
 		// RealConnection is not null if the app server yanks a local
 		// connection from one client and give it to another.  In this case,
 		// the real connection is ready to be used.  Otherwise, set it up
@@ -171,6 +166,12 @@
 			resetRealConnection();
 		}
 
+        // Need to do this in case the connection is forcibly removed without
+        // first being closed. Must be performed after resetRealConnection(),
+        // otherwise closing the logical connection may fail if the transaction
+        // is not idle.
+        closeCurrentConnectionHandle();
+
 		// now make a brokered connection wrapper and give this to the user
 		// we reuse the EmbedConnection(ie realConnection).
 		Connection c = getNewCurrentConnectionHandle();		
@@ -452,6 +453,13 @@
 	public void checkCommit() throws SQLException {
 	}
 
+    /** @see BrokeredConnectionControl#checkClose() */
+    public void checkClose() throws SQLException {
+        if (realConnection != null) {
+            realConnection.checkForTransactionInProgress();
+        }
+    }
+
 	/**
 		Close called on BrokeredConnection. If this call
 		returns true then getRealConnection().close() will be called.

Modified: db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection.java Fri Jul 11
01:55:29 2008
@@ -53,6 +53,16 @@
                 xaRes = new EmbedXAResource (this, ra);
 	}
 
+    /**
+     * Check if this connection is part of a global XA transaction.
+     *
+     * @return {@code true} if the transaction is global, {@code false} if the
+     * transaction is local
+     */
+    private boolean isGlobal() {
+        return xaRes.getCurrentXid () != null;
+    }
+
 	/*
 	** XAConnection methods
 	*/
@@ -69,7 +79,7 @@
 		Allow control over setting auto commit mode.
 	*/
 	public void checkAutoCommit(boolean autoCommit) throws SQLException {
-		if (autoCommit && (xaRes.getCurrentXid () != null))
+		if (autoCommit && isGlobal())
 			throw Util.generateCsSQLException(SQLState.CANNOT_AUTOCOMMIT_XA);
 
 		super.checkAutoCommit(autoCommit);
@@ -86,7 +96,7 @@
         throws SQLException
     {
 		if (holdability == ResultSet.HOLD_CURSORS_OVER_COMMIT) {		
-			if (xaRes.getCurrentXid () != null) {
+			if (isGlobal()) {
                 if (!downgrade)
                     throw Util.generateCsSQLException(SQLState.CANNOT_HOLD_CURSOR_XA);
                 
@@ -102,7 +112,7 @@
 	*/
 	public void checkSavepoint() throws SQLException {
 
-		if (xaRes.getCurrentXid () != null)
+		if (isGlobal())
 			throw Util.generateCsSQLException(SQLState.CANNOT_ROLLBACK_XA);
 
 		super.checkSavepoint();
@@ -113,7 +123,7 @@
 	*/
 	public void checkRollback() throws SQLException {
 
-		if (xaRes.getCurrentXid () != null)
+		if (isGlobal())
 			throw Util.generateCsSQLException(SQLState.CANNOT_ROLLBACK_XA);
 
 		super.checkRollback();
@@ -123,18 +133,32 @@
 	*/
 	public void checkCommit() throws SQLException {
 
-		if (xaRes.getCurrentXid () != null)
+		if (isGlobal())
 			throw Util.generateCsSQLException(SQLState.CANNOT_COMMIT_XA);
 
 		super.checkCommit();
 	}
 
+    /**
+     * @see org.apache.derby.iapi.jdbc.BrokeredConnectionControl#checkClose()
+     */
+    public void checkClose() throws SQLException {
+        if (isGlobal()) {
+            // It is always OK to close a connection associated with a global
+            // XA transaction, even if it isn't idle, since we still can commit
+            // or roll back the global transaction with the XAResource after
+            // the connection has been closed.
+        } else {
+            super.checkClose();
+        }
+    }
+
 	public Connection getConnection() throws SQLException
 	{
 		Connection handle;
 
 		// Is this just a local transaction?
-		if (xaRes.getCurrentXid () == null) {
+		if (!isGlobal()) {
 			handle = super.getConnection();
 		} else {
 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/J2EEDataSourceTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/J2EEDataSourceTest.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/J2EEDataSourceTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/J2EEDataSourceTest.java
Fri Jul 11 01:55:29 2008
@@ -136,6 +136,12 @@
         suite.addTest(new J2EEDataSourceTest("testSetIsolationWithStatement"));
         suite.addTest(new J2EEDataSourceTest("testJira95xads"));
         suite.addTest(new J2EEDataSourceTest("testBadConnectionAttributeSyntax"));
+        suite.addTest(new J2EEDataSourceTest("testCloseActiveConnection_DS"));
+        suite.addTest(new J2EEDataSourceTest("testCloseActiveConnection_CP"));
+        suite.addTest(
+            new J2EEDataSourceTest("testCloseActiveConnection_XA_local"));
+        suite.addTest(
+            new J2EEDataSourceTest("testCloseActiveConnection_XA_global"));
         suite.addTest(new J2EEDataSourceTest("testDescriptionProperty"));
         suite.addTest(new J2EEDataSourceTest("testConnectionErrorEvent"));
         suite.addTest(new J2EEDataSourceTest(
@@ -669,6 +675,109 @@
     }
 
     /**
+     * Test that {@code Connection.close()} behaves as expected when the
+     * transaction is active (DERBY-3319).
+     *
+     * @param c the connection to test
+     * @param autoCommit the expected auto-commit value. When auto-commit is
+     * on, {@code close()} shouldn't fail when the transaction is active.
+     * @param global tells whether the connection is part of a global XA
+     * transaction. If it is, {@code close()} shouldn't fail, since the
+     * transaction can be finished later without using the connection.
+     */
+    private void testCloseActiveConnection(Connection c, boolean autoCommit,
+                                           boolean global)
+        throws SQLException
+    {
+        if (global) {
+            assertFalse("auto-commit should be false in XA", autoCommit);
+        }
+        assertEquals("auto-commit", autoCommit, c.getAutoCommit());
+        Statement s = c.createStatement();
+        JDBC.assertDrainResults(s.executeQuery("SELECT * FROM SYS.SYSTABLES"));
+        s.close();
+        try {
+            c.close();
+            // should not fail in auto-commit or global XA, but should fail
+            // otherwise
+            assertTrue("close() should fail", autoCommit || global);
+        } catch (SQLException e) {
+            // no exception expected in auto-commit or global XA, re-throw
+            if (autoCommit || global) {
+                throw e;
+            }
+            assertSQLState("25001", e);
+        }
+        if (!autoCommit && !global) {
+            c.rollback();
+        }
+        c.close();
+    }
+
+    /**
+     * Test that connections retrieved from {@code DataSource} behave as
+     * expected when {@code close()} is called and the transaction is active.
+     */
+    public void testCloseActiveConnection_DS() throws SQLException {
+        DataSource ds = JDBCDataSource.getDataSource();
+        testCloseActiveConnection(ds.getConnection(), true, false);
+        Connection c = ds.getConnection();
+        c.setAutoCommit(false);
+        testCloseActiveConnection(c, false, false);
+    }
+
+    /**
+     * Test that connections retrieved from {@code ConnectionPoolDataSource}
+     * behave as expected when {@code close()} is called and the transaction is
+     * active.
+     */
+    public void testCloseActiveConnection_CP() throws SQLException {
+        ConnectionPoolDataSource ds =
+            J2EEDataSource.getConnectionPoolDataSource();
+        PooledConnection pc = ds.getPooledConnection();
+        testCloseActiveConnection(pc.getConnection(), true, false);
+        Connection c = pc.getConnection();
+        c.setAutoCommit(false);
+        testCloseActiveConnection(c, false, false);
+    }
+
+    /**
+     * Test that connections retrieved from {@code XADataSource} that are not
+     * part of a global XA transaction, behave as expected when {@code close()}
+     * is called and the transaction is active.
+     */
+    public void testCloseActiveConnection_XA_local() throws SQLException {
+        XADataSource ds = J2EEDataSource.getXADataSource();
+        XAConnection xa = ds.getXAConnection();
+        testCloseActiveConnection(xa.getConnection(), true, false);
+        Connection c = xa.getConnection();
+        c.setAutoCommit(false);
+        testCloseActiveConnection(c, false, false);
+    }
+
+    /**
+     * Test that connections retrieved from {@code XADataSource} that are part
+     * of a global XA transaction, behave as expected when {@code close()} is
+     * called and the transaction is active.
+     */
+    public void testCloseActiveConnection_XA_global()
+        throws SQLException, XAException
+    {
+        XADataSource ds = J2EEDataSource.getXADataSource();
+        XAConnection xa = ds.getXAConnection();
+        XAResource xar = xa.getXAResource();
+        Xid xid = new cdsXid(1, (byte) 2, (byte) 3);
+        xar.start(xid, XAResource.TMNOFLAGS);
+        // auto-commit is always off in XA transactions, so we expect
+        // getAutoCommit() to return false without having set it explicitly
+        testCloseActiveConnection(xa.getConnection(), false, true);
+        Connection c = xa.getConnection();
+        c.setAutoCommit(false);
+        testCloseActiveConnection(c, false, true);
+        xar.end(xid, XAResource.TMSUCCESS);
+    }
+
+    /**
      * Test that a PooledConnection can be reused and closed
      * (separately) during the close event raised by the
      * closing of its logical connection.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java?rev=675870&r1=675869&r2=675870&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/StatementPoolingTest.java
Fri Jul 11 01:55:29 2008
@@ -359,7 +359,7 @@
         Statement stmt = createStatement();
         stmt.executeUpdate("create table clcclso (id int)");
         PreparedStatement ps = con.prepareStatement(SELECT_SQL);
-        commit();
+        con.commit();
         con.close();
         try {
             // Should fail because the logical statement has been closed.
@@ -586,6 +586,7 @@
         if (holdability == ResultSet.HOLD_CURSORS_OVER_COMMIT) {
             assertTrue(rs.next());
             assertEquals(2, rs.getInt(1));
+            rollback();
         }
         getConnection().close();
         try {
@@ -829,9 +830,8 @@
                 "resTestNoCommitOnReuse"));
         reqDataSuite.addTest(new StatementPoolingTest(
                 "resTestCommitOnReuse"));
-        // This test fails, DERBY-3319 is probably the cause.
-        //reqDataSuite.addTest(new StatementPoolingTest(
-        //        "resTestNoDataCommittedOnInvalidTransactionState"));
+        reqDataSuite.addTest(new StatementPoolingTest(
+                "resTestNoDataCommittedOnInvalidTransactionState"));
         suite.addTest(TestConfiguration.connectionCPDecorator(
                 new BaseJDBCTestSetup(reqDataSuite) {
                 public void setUp() throws Exception {



Mime
View raw message