JBTM-2676 JDBC close handling

jbosstm · Sep 22, 2016 · 2912b58 · 2912b58
1 parent c1bbf4f
commit 2912b58
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 69 deletions.
diff --git a/ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/ConnectionImple.java b/ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/ConnectionImple.java
@@ -295,19 +295,9 @@ public void rollback() throws SQLException
 	 */
 	public void close() throws SQLException
 	{
+		boolean delayClose = false;
 	    try
 	    {
-	        /*
-	         * Need to know whether this particular connection has outstanding
-	         * resources waiting for it. If not then we can close, otherwise we
-	         * can't.
-	         */
-
-	        if (!_transactionalDriverXAConnectionConnection.inuse())
-	        {
-	            ConnectionManager.remove(this); // finalize?
-	        }
-
 	        /*
 	         * Delist resource if within a transaction.
 	         */
@@ -322,8 +312,6 @@ public void close() throws SQLException
 	         * connection is enlisted with!
 	         */
 
-	        boolean delayClose = false;
-
 	        if (tx != null)
 	        {
 	            if (_transactionalDriverXAConnectionConnection.validTransaction(tx))
@@ -350,6 +338,7 @@ public void close() throws SQLException
 	                     * 
 	                     * JBTM-789.
 	                     */
+	                    delayClose = true;
 	                }
 	                else
 	                {
@@ -366,31 +355,15 @@ public void close() throws SQLException
 	                    }
 	                }
 
-	                tx.registerSynchronization(new ConnectionSynchronization(null, _transactionalDriverXAConnectionConnection));
 	                if (delayClose)
 	                {
-        	                tx.registerSynchronization(new ConnectionSynchronization(_theConnection, null));
-
-                                _theConnection = null;
+        	                tx.registerSynchronization(new ConnectionSynchronization(this));
 	                }
 	            }
 	            else
 	                throw new SQLException(jdbcLogger.i18NLogger.get_closeerrorinvalidtx(tx.toString()));
-	        } else {
-	            _transactionalDriverXAConnectionConnection.closeCloseCurrentConnection();
 	        }
-
-	        if (!delayClose)  // close now
-	        {
-	            if (!_theConnection.isClosed()) {
-	                _theConnection.close();
-	            }
-
-	            _theConnection = null;
-	        }
-
-	        // what about connections without xaCon?
-	    }
+		}
 	    catch (IllegalStateException ex)
 	    {
 	        // transaction not running, so ignore.
@@ -404,7 +377,26 @@ public void close() throws SQLException
 	        SQLException sqlException = new SQLException(jdbcLogger.i18NLogger.get_closeerror());
 	        sqlException.initCause(e1);
 	        throw sqlException;
-	    }
+	    } finally {
+			if (!delayClose) {
+				closeImpl();
+			}
+		}
+	}
+
+	void closeImpl() throws SQLException {
+		try {
+			ConnectionManager.remove(this);
+			if (!_theConnection.isClosed()) {
+				_theConnection.close();
+			}
+			if (_transactionalDriverXAConnectionConnection != null) {
+				_transactionalDriverXAConnectionConnection.closeCloseCurrentConnection();
+			}
+		} finally {
+			_theConnection = null;
+			_transactionalDriverXAConnectionConnection = null;
+		}
 	}
 
 	public boolean isClosed() throws SQLException
@@ -414,8 +406,6 @@ public boolean isClosed() throws SQLException
 		 * bound it to a different transaction.
 		 */
 
-		checkTransaction();
-
 		if (_theConnection == null)
 			return false; // not opened yet. // TODO why don't we return true here
 		else
@@ -860,16 +850,11 @@ final void reset()
 	{
 		try
 		{
-			if (_theConnection != null)
-				_theConnection.close();
+			closeImpl();
 		}
 		catch (Exception ex)
 		{
 		}
-		finally
-		{
-			_theConnection = null;
-		}
 	}
 
 	final java.sql.Connection getConnection() throws SQLException

diff --git a/ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/ConnectionSynchronization.java b/ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/ConnectionSynchronization.java
@@ -31,12 +31,10 @@
 
 package com.arjuna.ats.internal.jdbc;
 
-import java.sql.Connection;
+import com.arjuna.ats.jdbc.logging.jdbcLogger;
 
 import javax.transaction.Synchronization;
 
-import com.arjuna.ats.jta.xa.RecoverableXAConnection;
-
 /**
  * A synchronization to close the database connection when the transaction
  * has committed or rolled back.
@@ -47,39 +45,29 @@
 public class ConnectionSynchronization implements Synchronization
 {
 
-    public ConnectionSynchronization (Connection conn, TransactionalDriverXAConnection rxac)
+    public ConnectionSynchronization (ConnectionImple conn)
     {
 	_theConnection = conn;
-	_recoveryConnection = rxac;
     }
 
     public void afterCompletion(int status)
     {
-	try
-	{
-	    if (_theConnection != null && !_theConnection.isClosed()) {
-	        _theConnection.close();
-	    }
-	}
-	catch (Exception ex)
-	{
-	}
-	try
-	{
-	    if (_recoveryConnection != null) {
-	        _recoveryConnection.closeCloseCurrentConnection();
-	    }
-	}
-	catch (Exception ex)
-	{
-	}
+		try
+		{
+			if (_theConnection != null) {
+				_theConnection.closeImpl();
+			}
+		}
+		catch (Exception ex)
+		{
+			jdbcLogger.logger.warn("ConnectionSynchronization could not close connection", ex);
+		}
     }
 
     public void beforeCompletion()
     {
     }
 
-    private Connection _theConnection = null;
-    private TransactionalDriverXAConnection _recoveryConnection;
+    private ConnectionImple _theConnection = null;
 }
 
diff --git a/ArjunaJTA/jdbc/tests/classes/com/hp/mwtests/ts/jdbc/basic/ConnectionImpleTest.java b/ArjunaJTA/jdbc/tests/classes/com/hp/mwtests/ts/jdbc/basic/ConnectionImpleTest.java
@@ -42,6 +42,7 @@
 import java.sql.SQLException;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -112,7 +113,6 @@ public void checkIfConnectionIsClosedWithTransaction() throws Exception {
     }
 
     @Test
-    @Ignore // https://issues.jboss.org/browse/JBTM-2676
     public void checkIfConnectionIsClosedInAfterCompletion() throws Exception {
         transactionManager.begin();
         Connection connectionToTest = getConnectionToTest();
@@ -125,13 +125,14 @@ public void beforeCompletion() { }
             public void afterCompletion(int status) {
                 try {
                     connectionToTest.isClosed();
+                    connectionToTest.close();
                 } catch (SQLException e) {
-                    e.printStackTrace();
+                    fail("Could not close connection: " + e.getMessage());
                 }
             }
         });
         transactionManager.commit();
-        verify(connection, times(1)).isClosed();
+        verify(connection, times(2)).isClosed();
     }
 
     @Test
@@ -156,7 +157,6 @@ public void closeConnectionWithTransaction() throws Exception {
      * This test currently fails because of https://issues.jboss.org/browse/JBTM-2676
      */
     @Test
-    @Ignore // https://issues.jboss.org/browse/JBTM-2676
     public void closeConnectionInAfterCompletion() throws Exception {
         transactionManager.begin();
         transactionManager.getTransaction().enlistResource(xaResource); // Normally driver does this