You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by bp...@apache.org on 2016/07/09 00:05:04 UTC

svn commit: r1751977 - in /db/derby/code/trunk/java: build/org/apache/derbyBuild/lastgoodjarcontents/ engine/org/apache/derby/jdbc/ testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/

Author: bpendleton
Date: Sat Jul  9 00:05:04 2016
New Revision: 1751977

URL: http://svn.apache.org/viewvc?rev=1751977&view=rev
Log:
DERBY-6879: Engine deadlock between XA timeout handling and cleanupOnError

This patch was contributed by Brett Bergquist (brett at thebergquistfamily dot com)

The original problem to be solved is that a connection that is performing a
XA transaction that discovers an error that must be cleaned up is going to
have a lock on a EmbedConnection and will then require a lock on the
XATransactionState.

And if the same XA transaction times out (because of the XA transaction
timer value) while the original XA transaction is being worked, then the
timeout handling is going to have a lock on the XATransactionState and then
require a lock on the EmbedConnection, triggering the deadlock.

The change in the patch fixes this problem by altering the locking on
the XATransactionState when invoked via the timeout by removing the
synchronization on the "cancel" method and instead using inline
synchronization on the "XATransactionState" while its internals are being
altered.

Then, it releases the XATransactionState lock to allow the
EmbedConnection.rollback method to be invoked without the lock.

Finally, it acquires the lock again to finish cleaning up the XATransactionState.

Modified:
    db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/insane.derbyTesting.jar.lastcontents
    db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/sane.derbyTesting.jar.lastcontents
    db/derby/code/trunk/java/engine/org/apache/derby/jdbc/XATransactionState.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.java

Modified: db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/insane.derbyTesting.jar.lastcontents
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/insane.derbyTesting.jar.lastcontents?rev=1751977&r1=1751976&r2=1751977&view=diff
==============================================================================
--- db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/insane.derbyTesting.jar.lastcontents (original)
+++ db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/insane.derbyTesting.jar.lastcontents Sat Jul  9 00:05:04 2016
@@ -738,6 +738,7 @@ org.apache.derbyTesting.functionTests.ut
 org.apache.derbyTesting.functionTests.util.BigDecimalHandler.class
 org.apache.derbyTesting.functionTests.util.CanonTestCase.class
 org.apache.derbyTesting.functionTests.util.DbFile.class
+org.apache.derbyTesting.functionTests.util.DeadlockWatchdog.class
 org.apache.derbyTesting.functionTests.util.DerbyJUnitTest.class
 org.apache.derbyTesting.functionTests.util.ExtendingInterface.class
 org.apache.derbyTesting.functionTests.util.FTFileUtil.class

Modified: db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/sane.derbyTesting.jar.lastcontents
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/sane.derbyTesting.jar.lastcontents?rev=1751977&r1=1751976&r2=1751977&view=diff
==============================================================================
--- db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/sane.derbyTesting.jar.lastcontents (original)
+++ db/derby/code/trunk/java/build/org/apache/derbyBuild/lastgoodjarcontents/sane.derbyTesting.jar.lastcontents Sat Jul  9 00:05:04 2016
@@ -738,6 +738,7 @@ org.apache.derbyTesting.functionTests.ut
 org.apache.derbyTesting.functionTests.util.BigDecimalHandler.class
 org.apache.derbyTesting.functionTests.util.CanonTestCase.class
 org.apache.derbyTesting.functionTests.util.DbFile.class
+org.apache.derbyTesting.functionTests.util.DeadlockWatchdog.class
 org.apache.derbyTesting.functionTests.util.DerbyJUnitTest.class
 org.apache.derbyTesting.functionTests.util.ExtendingInterface.class
 org.apache.derbyTesting.functionTests.util.FTFileUtil.class

Modified: db/derby/code/trunk/java/engine/org/apache/derby/jdbc/XATransactionState.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/jdbc/XATransactionState.java?rev=1751977&r1=1751976&r2=1751977&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/jdbc/XATransactionState.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/jdbc/XATransactionState.java Sat Jul  9 00:05:04 2016
@@ -107,16 +107,18 @@ final class XATransactionState extends C
             this.xaState = xaState;
         }
         
-        public boolean cancel() {
+        public synchronized boolean cancel() {
             // nullify reference to reduce memory footprint of canceled tasks
             xaState = null;
             return super.cancel();
         }
 
         /** Runs the cancel task of the global transaction */
-        public void run() {
+        public synchronized void run() {
             try {
-                xaState.cancel(MessageId.CONN_XA_TRANSACTION_TIMED_OUT);
+                if (null != xaState) {
+                    xaState.cancel(MessageId.CONN_XA_TRANSACTION_TIMED_OUT);
+                }
             } catch (Throwable th) {
                 Monitor.logThrowable(th);
             }
@@ -403,13 +405,20 @@ final class XATransactionState extends C
      *
      * @see CancelXATransactionTask
      */
-    synchronized void cancel(String messageId) throws XAException {
-        // Check performTimeoutRollback just to be sure that
-        // the cancellation task was not started
-        // just before the xa_commit/rollback
-        // obtained this object's monitor.
-        if (performTimeoutRollback) {
-
+    void cancel(String messageId) throws XAException {
+        // Note that the synchronization has changed for this method.   See
+        //  DERBY-6879.
+        //
+        boolean needsRollback = false;
+        
+        // This method now synchronizes on this instanace to ensure that the state
+        //  is consistent when accessed and modified.  See DERBY-6879
+        synchronized (this) {
+            // Check performTimeoutRollback just to be sure that
+            // the cancellation task was not started
+            // just before the xa_commit/rollback
+            // obtained this object's monitor.
+            needsRollback = this.performTimeoutRollback;
             // Log the message about the transaction cancelled
             if (messageId != null)
                 Monitor.logTextMessage(messageId, xid.toString());
@@ -421,21 +430,29 @@ final class XATransactionState extends C
                 EmbedXAResource assocRes = associatedResource;
                 end(assocRes, XAResource.TMFAIL, true);
             }
-
-            // Rollback the global transaction
+        }
+        if (needsRollback) {
+            // While the rollback is performed on the connection, 
+            //  this XATransactionState is ont synchronized to work around 
+            //  the issue reported in DERBY-6879
             try {
+                // Rollback the global transaction
                 conn.xa_rollback();
             } catch (SQLException sqle) {
                 XAException ex = new XAException(XAException.XAER_RMERR);
                 ex.initCause(sqle);
                 throw ex;
             }
+        }
 
+        // This method now synchronizes on this instanace again to ensure that the state
+        //  is consistent when accessed and modified.  See DERBY-6879
+        synchronized (this) {
             // Do the cleanup on the resource
             creatingResource.returnConnectionToResource(this, xid);
         }
     }
-    
+   
     /**
      * Privileged Monitor lookup. Must be private so that user code
      * can't call this entry point.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.java?rev=1751977&r1=1751976&r2=1751977&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.java Sat Jul  9 00:05:04 2016
@@ -34,18 +34,26 @@ import javax.sql.XADataSource;
 import javax.transaction.xa.XAException;
 import javax.transaction.xa.XAResource;
 import javax.transaction.xa.Xid;
+import static junit.framework.Assert.fail;
 import junit.framework.Test;
+import org.apache.derby.shared.common.reference.SQLState;
+import org.apache.derbyTesting.functionTests.util.DeadlockWatchdog;
 import org.apache.derbyTesting.junit.BaseJDBCTestCase;
+import static org.apache.derbyTesting.junit.BaseJDBCTestCase.assertSQLState;
 import org.apache.derbyTesting.junit.BaseTestSuite;
 import org.apache.derbyTesting.junit.CleanDatabaseTestSetup;
 import org.apache.derbyTesting.junit.DatabasePropertyTestSetup;
 import org.apache.derbyTesting.junit.J2EEDataSource;
 import org.apache.derbyTesting.junit.JDBC;
+import org.apache.derbyTesting.junit.SecurityManagerSetup;
 import org.apache.derbyTesting.junit.TestConfiguration;
 import org.apache.derbyTesting.junit.Utilities;
 import org.apache.derbyTesting.junit.XATestUtil;
 
 public class XATest extends BaseJDBCTestCase {
+    //create own policy file
+    private static final String POLICY_FILE_NAME =
+            "org/apache/derbyTesting/functionTests/tests/jdbcapi/XATest.policy";
 
     public static final String LOCKTIMEOUT="40XL1";
     
@@ -1224,6 +1232,70 @@ public class XATest extends BaseJDBCTest
         
     }
     
+    /**
+     * DERBY-6879 Check that a XA transaction timeout while a cleanupOnError is
+     * being performed does not cause a Java level deadlock.
+     *
+     * The strategy is to cause a XA statement to wait for a period that is
+     * longer than the XA transaction timeout which will allow the timeout to
+     * cancel the XA transaction. That is accomplished by locking a table using
+     * a standard connection and then issuing a select on the same table using a
+     * XA connection. The select will wait for the required locks to be
+     * available up to the lock timeout. For the test to work correctly the XA
+     * transaction timeout must be less than the lock timeout.
+     *
+     * A dealock watchdog is used to detect the deadlock until the DERBY-6879
+     * issue is fixed.
+     *
+     * @throws SQLException
+     * @throws XAException
+     */
+    public void testDerby6879() throws SQLException, XAException {
+        XADataSource xads = J2EEDataSource.getXADataSource();
+        J2EEDataSource.setBeanProperty(xads, "databaseName", "wombat");
+
+        Connection conn = J2EEDataSource.getConnectionPoolDataSource().getPooledConnection().getConnection();
+        conn.setAutoCommit(false);
+        Statement s = conn.createStatement();
+        s.execute("LOCK TABLE TABLT IN EXCLUSIVE MODE");
+
+        // Get a second connection and global xact
+        // and try to select causing lock timeout
+        XAConnection xaconn2 = xads.getXAConnection();
+        XAResource xar2 = xaconn2.getXAResource();
+        xar2.setTransactionTimeout(2);
+        Xid xid2 = XATestUtil.getXid(6879, 11, 51);
+        Connection conn2 = xaconn2.getConnection();
+        // Set to serializable so we get lock timeout
+        conn2.setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE);
+        xar2.start(xid2, XAResource.TMNOFLAGS);
+        Statement s2 = conn2.createStatement();
+        assertGlobalXactCount(1);
+        DeadlockWatchdog wd = new DeadlockWatchdog(30 * 1000);
+        wd.start();
+        try {
+            ResultSet rs = s2.executeQuery("SELECT * FROM TABLT");
+            fail("Should have gotten lock timeout error: " + LOCKTIMEOUT);
+        } catch (SQLException se) {
+            assertSQLState(LOCKTIMEOUT, se);
+        }
+        wd.stop();
+
+        // xid2 should have already been rolled back so end should fail
+        try {
+            xar2.end(xid2, XAResource.TMSUCCESS);
+            fail("Should have gotten exception ending xid2");
+        } catch (XAException xae) {
+            assertTrue(xae.errorCode >= XAException.XAER_OUTSIDE || xae.errorCode <= XAException.XAER_ASYNC);
+        }
+
+        conn.commit();
+
+        conn.close();
+        conn2.close();
+        xaconn2.close();
+    }
+    
     
     /**
      * The two cases for DERBY-4371 do essentially the same thing. Except doing
@@ -1430,7 +1502,22 @@ public class XATest extends BaseJDBCTest
         suite.addTest(TestConfiguration
                 .clientServerDecorator(baseSuite("XATest:client")));
         
-        return DatabasePropertyTestSetup.setLockTimeouts(suite, 3, 5);
+        Test test = DatabasePropertyTestSetup.setLockTimeouts(suite, 3, 5);
+        test = decorateWithPolicy(test);
+        return test;
+    }   
+    
+    // grant ALL FILES execute, and getPolicy permissions,
+    // as well as write for the trace files.
+    private static Test decorateWithPolicy(Test test) {
+        //
+        // Install a security manager using the initial policy file. This is 
+        // needed foro the DeadlockWatchdog to allow it to access the ThreadMXBean
+        // to check for a deadlock
+        //
+        return new SecurityManagerSetup(test, POLICY_FILE_NAME);
     }
-
+    
+    
 }
+