JBTM-2676 JDBC close handling

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))
@@ -366,31 +354,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 +376,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 +405,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 +849,11 @@ final void reset()
 	{
 		try
 		{
-			if (_theConnection != null)
-				_theConnection.close();
+			closeImpl();
 		}
 		catch (Exception ex)
 		{
 		}
-		finally
-		{
-			_theConnection = null;
-		}
 	}
 
 	final java.sql.Connection getConnection() throws SQLException

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;
 }
 

ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/drivers/modifiers/IsSameRMModifier.java

@@ -62,19 +62,19 @@ public Xid createXid(Xid xid) throws SQLException, NotImplementedException {
 	@Override
 	public XAConnection getConnection(XAConnection conn) throws SQLException,
 			NotImplementedException {
-		throw new NotImplementedException();
+		throw new NotImplementedException(); // NEVER CALLED
 	}
 
 	@Override
 	public boolean supportsMultipleConnections() throws SQLException,
 			NotImplementedException {
-		return true; // This is the default
+		return true; // This ensures connection close is delayed
 	}
 
 	@Override
 	public void setIsolationLevel(Connection conn, int level)
 			throws SQLException, NotImplementedException {
-		// The default implementation does not call this
+		// Non-modifier path does not call this
 	}
 
 	@Override

ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/drivers/modifiers/SupportsMultipleConnectionsModifier.java

@@ -0,0 +1,89 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2006, Red Hat Middleware LLC, and individual contributors 
+ * as indicated by the @author tags. 
+ * See the copyright.txt in the distribution for a
+ * full listing of individual contributors. 
+ * This copyrighted material is made available to anyone wishing to use,
+ * modify, copy, or redistribute it subject to the terms and conditions
+ * of the GNU Lesser General Public License, v. 2.1.
+ * This program is distributed in the hope that it will be useful, but WITHOUT A 
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A 
+ * PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more details.
+ * You should have received a copy of the GNU Lesser General Public License,
+ * v.2.1 along with this distribution; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, 
+ * MA  02110-1301, USA.
+ * 
+ * (C) 2005-2006,
+ * @author JBoss Inc.
+ */
+/*
+ * Copyright (C) 1998, 1999, 2000,
+ *
+ * Arjuna Solutions Limited,
+ * Newcastle upon Tyne,
+ * Tyne and Wear,
+ * UK.  
+ *
+ * $Id: oracle_9_0.java 2342 2006-03-30 13:06:17Z  $
+ */
+
+package com.arjuna.ats.internal.jdbc.drivers.modifiers;
+
+import com.arjuna.ats.jta.exceptions.NotImplementedException;
+import com.arjuna.ats.jta.xa.XAModifier;
+
+import javax.sql.XAConnection;
+import javax.transaction.xa.Xid;
+import java.sql.Connection;
+import java.sql.SQLException;
+
+/*
+ * This is a stateless class to allow us to get round
+ * problems in Oracle. For example, they can't work with
+ * an arbitrary implementation of Xid - it has to be their
+ * own implementation!
+ */
+
+public class SupportsMultipleConnectionsModifier implements XAModifier, ConnectionModifier {
+
+	@Override
+	public String initialise(String dbName) {
+		return dbName;
+	}
+
+	@Override
+	public Xid createXid(Xid xid) throws SQLException, NotImplementedException {
+		return xid;
+	}
+
+	@Override
+	public XAConnection getConnection(XAConnection conn) throws SQLException,
+			NotImplementedException {
+		throw new NotImplementedException(); // NEVER CALLED
+	}
+
+	@Override
+	public boolean supportsMultipleConnections() throws SQLException,
+			NotImplementedException {
+		return true; // This ensures connection close is delayed
+	}
+
+	@Override
+	public void setIsolationLevel(Connection conn, int level)
+			throws SQLException, NotImplementedException {
+		// Non-modifier path does not call this
+	}
+
+	@Override
+	public int xaStartParameters(int level) throws SQLException,
+			NotImplementedException {
+		return level;
+	}
+
+	@Override
+	public boolean requiresSameRMOverride() {
+		return false;
+	}
+}

ArjunaJTA/jdbc/classes/com/arjuna/ats/internal/jdbc/drivers/modifiers/list.java

@@ -39,9 +39,13 @@ public list ()
 				"Oracle JDBC driver",
 				"IBM DB2 JDBC Universal Driver Architecture",
 				"MySQL Connector Java",
+				"MySQL-AB JDBC Driver",
 				"H2 JDBC Driver"}) {
 			ModifierFactory.putModifier(driver, -1, -1,
 					IsSameRMModifier.class.getName());
 		}
-    }
+
+		ModifierFactory.putModifier("PostgreSQL Native Driver", -1, -1,
+				SupportsMultipleConnectionsModifier.class.getName());
+	}
 }

ArjunaJTA/jdbc/tests/classes/com/hp/mwtests/ts/jdbc/basic/ConnectionImpleTest.java

@@ -26,7 +26,6 @@
 import com.arjuna.ats.internal.jdbc.ConnectionImple;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -42,6 +41,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 +112,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 +124,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 +156,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