diff --git a/framework/src/main/groovy/org/moqui/impl/context/ContextJavaUtil.java b/framework/src/main/groovy/org/moqui/impl/context/ContextJavaUtil.java index f3054aec4..64256cb5a 100644 --- a/framework/src/main/groovy/org/moqui/impl/context/ContextJavaUtil.java +++ b/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(); } diff --git a/framework/src/main/groovy/org/moqui/impl/context/TransactionFacadeImpl.groovy b/framework/src/main/groovy/org/moqui/impl/context/TransactionFacadeImpl.groovy index 755251634..914197ddc 100644 --- a/framework/src/main/groovy/org/moqui/impl/context/TransactionFacadeImpl.groovy +++ b/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()) } + } } }