Fix zombie XA connection after deadlock by deferring closeTxConnections

framework/src/main/groovy/org/moqui/impl/context/ContextJavaUtil.java

@@ -40,8 +40,11 @@
 
 import jakarta.transaction.Synchronization;
 import jakarta.transaction.Transaction;
+import javax.transaction.xa.XAException;
 import javax.transaction.xa.XAResource;
 import java.io.IOException;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Proxy;
 import java.math.BigDecimal;
 import java.math.RoundingMode;
 import java.sql.*;
@@ -471,6 +474,9 @@ public static class ConnectionWrapper implements Connection {
         protected Connection con;
         TransactionFacadeImpl tfi;
         String groupName;
+        // When XAER_RMFAIL is detected on this connection, mark it for physical destruction on close
+        // so the dirty XA branch is not returned to the Bitronix pool and recycled indefinitely.
+        private volatile boolean destroyOnClose = false;
 
         public ConnectionWrapper(Connection con, TransactionFacadeImpl tfi, String groupName) {
             this.con = con;
@@ -481,12 +487,62 @@ public ConnectionWrapper(Connection con, TransactionFacadeImpl tfi, String group
         public String getGroupName() { return groupName; }
 
         public void closeInternal() throws SQLException {
-            con.close();
+            if (destroyOnClose) {
+                // Physical destroy: removes connection from Bitronix pool entirely.
+                // Prevents XAER_RMFAIL cascade: dirty XA branch stays on pool connection when
+                // release() is called (XA END no-op + requeue), causing every subsequent checkout
+                // to fail. JdbcPooledConnection.close() destroys the physical connection instead.
+                physicallyDestroy();
+            } else {
+                // Normal path: release() back to pool via ConnectionJavaProxy.close().
+                con.close();
+            }
         }
 
-        @Override public Statement createStatement() throws SQLException { return con.createStatement(); }
-        @Override public PreparedStatement prepareStatement(String sql) throws SQLException { return con.prepareStatement(sql); }
-        @Override public CallableStatement prepareCall(String sql) throws SQLException { return con.prepareCall(sql); }
+        private void physicallyDestroy() {
+            // con is a JDK Proxy; handler is ConnectionJavaProxy which has getPooledConnection().
+            // JdbcPooledConnection.close() unregisters from pool and closes the physical DB connection.
+            try {
+                if (Proxy.isProxyClass(con.getClass())) {
+                    InvocationHandler handler = Proxy.getInvocationHandler(con);
+                    Object jdbcPC = handler.getClass().getMethod("getPooledConnection").invoke(handler);
+                    jdbcPC.getClass().getMethod("close").invoke(jdbcPC);
+                    return;
+                }
+            } catch (Throwable t) {
+                logger.warn("Could not physically destroy Bitronix pooled connection for group " + groupName + ", falling back to close()", t);
+            }
+            try { con.close(); } catch (Throwable t) { logger.warn("Error on fallback close for group " + groupName, t); }
+        }
+
+        private static boolean isXaRmFail(Throwable e) {
+            for (Throwable t = e; t != null; t = t.getCause()) {
+                if (t instanceof XAException && ((XAException) t).errorCode == XAException.XAER_RMFAIL) return true;
+            }
+            return false;
+        }
+
+        @Override public Statement createStatement() throws SQLException {
+            try { return con.createStatement(); }
+            catch (SQLException e) {
+                if (isXaRmFail(e)) destroyOnClose = true;
+                throw e;
+            }
+        }
+        @Override public PreparedStatement prepareStatement(String sql) throws SQLException {
+            try { return con.prepareStatement(sql); }
+            catch (SQLException e) {
+                if (isXaRmFail(e)) destroyOnClose = true;
+                throw e;
+            }
+        }
+        @Override public CallableStatement prepareCall(String sql) throws SQLException {
+            try { return con.prepareCall(sql); }
+            catch (SQLException e) {
+                if (isXaRmFail(e)) destroyOnClose = true;
+                throw e;
+            }
+        }
         @Override public String nativeSQL(String sql) throws SQLException { return con.nativeSQL(sql); }
         @Override public void setAutoCommit(boolean autoCommit) throws SQLException { con.setAutoCommit(autoCommit); }
         @Override public boolean getAutoCommit() throws SQLException { return con.getAutoCommit(); }

framework/src/main/groovy/org/moqui/impl/context/TransactionFacadeImpl.groovy

@@ -395,7 +395,6 @@ class TransactionFacadeImpl implements TransactionFacade {
             int status = ut.getStatus()
             // logger.warn("================ commit TX, currentStatus=${status}")
 
-            txStackInfo.closeTxConnections()
             if (status == Status.STATUS_MARKED_ROLLBACK) {
                 if (txStackInfo.rollbackOnlyInfo != null) {
                     logger.warn("Tried to commit transaction but marked rollback only, doing rollback instead; rollback-only was set here:", txStackInfo.rollbackOnlyInfo.rollbackLocation)
@@ -411,6 +410,8 @@ class TransactionFacadeImpl implements TransactionFacade {
                 if (status != Status.STATUS_NO_TRANSACTION)
                     logger.warn((String) "Not committing transaction because status is " + getStatusString(), new Exception("Bad TX status location"))
             }
+            // closeTxConnections() is not called here; clearCurrent() in finally handles it after the
+            // JTA operation completes, so connections are never released before XA END + XA COMMIT/ROLLBACK.
         } catch (RollbackException e) {
             if (txStackInfo.rollbackOnlyInfo != null) {
                 logger.warn("Could not commit transaction, was marked rollback-only. The rollback-only was set here: ", txStackInfo.rollbackOnlyInfo.rollbackLocation)
@@ -455,7 +456,6 @@ class TransactionFacadeImpl implements TransactionFacade {
         if (ut == null) throw new IllegalStateException("No transaction manager in place")
         TxStackInfo txStackInfo = getTxStackInfo()
         try {
-            txStackInfo.closeTxConnections()
 
             // logger.warn("================ rollback TX, currentStatus=${getStatus()}")
             if (getStatus() == Status.STATUS_NO_TRANSACTION) {
@@ -476,6 +476,12 @@ class TransactionFacadeImpl implements TransactionFacade {
             }
 
             ut.rollback()
+            // closeTxConnections() is intentionally NOT called here. clearCurrent() in the finally block
+            // calls it after ut.rollback() completes, ensuring BTM can issue XA END + XA ROLLBACK on all
+            // enlisted resources while they are still tracked. Calling it before ut.rollback() (the previous
+            // behavior) could leave XA connections in a zombie ROLLBACK_ONLY state in the pool when a MySQL
+            // deadlock (XA_RBDEADLOCK) occurs, causing XAER_RMFAIL on the next transaction that acquires
+            // the same connection.
         } catch (IllegalStateException e) {
             throw new TransactionException("Could not rollback transaction", e)
         } catch (SystemException e) {
@@ -532,10 +538,6 @@ class TransactionFacadeImpl implements TransactionFacade {
                 return false
             }
 
-            // close connections before suspend, let the pool reuse them
-            TxStackInfo txStackInfo = getTxStackInfo()
-            txStackInfo.closeTxConnections()
-
             Transaction tx = tm.suspend()
             // only do these after successful suspend
             pushTxStackInfo(tx, new Exception("Transaction Suspend Location"))
@@ -573,12 +575,24 @@ class TransactionFacadeImpl implements TransactionFacade {
     @Override
     Connection enlistConnection(XAConnection con) {
         if (con == null) return null
+        boolean enlisted = false
         try {
             XAResource resource = con.getXAResource()
             this.enlistResource(resource)
-            return con.getConnection()
+            // enlistResource succeeded: XA branch
+            Connection c = con.getConnection()
+            enlisted = true
+            return c
         } catch (SQLException e) {
             throw new TransactionException("Could not enlist connection in transaction", e)
+        } finally {
+            if (!enlisted) {
+                // Enlistment or connection retrieval failed. Close and destroy the XAConnection so
+                // Bitronix removes the physical connection from the pool. This prevents the cascade
+                // where a dirty XA branch (e.g. XAER_RMFAIL) is returned to the pool and triggers
+                // XAER_RMFAIL on every subsequent checkout of the same physical connection.
+                try { con.close() } catch (Throwable t) { logger.warn("Error closing XAConnection after enlist failure: " + t.getMessage()) }
+            }
         }
     }