Skip to content

Fix zombie XA connection after deadlock by deferring closeTxConnections - #725

Draft
dixitdeepak wants to merge 2 commits into
moqui:masterfrom
dixitdeepak:fix-zombie-XA-connection-after-deadlock
Draft

Fix zombie XA connection after deadlock by deferring closeTxConnections#725
dixitdeepak wants to merge 2 commits into
moqui:masterfrom
dixitdeepak:fix-zombie-XA-connection-after-deadlock

Conversation

@dixitdeepak

Copy link
Copy Markdown
Contributor

closeTxConnections() was called at the top of rollback() and commit(), before ut.rollback()/ut.commit().

When MySQL auto-rolled back an XA branch due to a deadlock (XA_RBDEADLOCK), BTM attempted XA END on the connection during close(), failed silently, and lost track of the XA resource. The connection was then returned to the pool with an unresolved ROLLBACK_ONLY XA branch still open in MySQL.

The next thread to acquire that connection received XAER_RMFAIL on XA START.

The fix removes the premature closeTxConnections() calls from both methods. clearCurrent() in the finally block already calls closeTxConnections() as a safety net — it now becomes the sole caller, always running after the JTA operation completes and the XA lifecycle (XA END + XA ROLLBACK/COMMIT) has been properly finalized on all enlisted resources. suspend() is unaffected as it intentionally closes connections before suspending.

   closeTxConnections() was called at the top of rollback() and commit(),
   before ut.rollback()/ut.commit(). When MySQL auto-rolled back an XA
   branch due to a deadlock (XA_RBDEADLOCK), BTM attempted XA END on the
   connection during close(), failed silently, and lost track of the XA
   resource. The connection was then returned to the pool with an unresolved
   ROLLBACK_ONLY XA branch still open in MySQL.

The next thread to acquire that connection received XAER_RMFAIL on XA
   START.
The fix removes the premature closeTxConnections() calls from both
   methods. clearCurrent() in the finally block already calls
   closeTxConnections() as a safety net — it now becomes the sole caller,
   always running after the JTA operation completes and the XA lifecycle
   (XA END + XA ROLLBACK/COMMIT) has been properly finalized on all
   enlisted resources. suspend() is unaffected as it intentionally closes
   connections before suspending.
@dixitdeepak

Copy link
Copy Markdown
Contributor Author

Previously, transaction connections could be closed before UserTransaction.rollback() was executed. This creates a risk that resources participating in the transaction are released before the transaction manager completes rollback processing, particularly in failure scenarios where the resource manager has already reported an error.

This change ensures that:

  1. UserTransaction.rollback() is executed first.
  2. Transaction-scoped connections and resources are cleaned up only after rollback processing completes.

This ordering allows the transaction manager to properly complete rollback processing on all enlisted resources before they are released, reducing the risk of resource state inconsistencies and subsequent transaction enlistment failures when connections are returned to and later reused from the pool.

…XA transactions

Root cause:
When MySQL marks an XA branch as rollback-only (e.g. after a deadlock or timeout),
Bitronix auto-enlistment fails with XAER_RMFAIL on the next statement call. Because
PoolingDataSource does not implement javax.sql.XADataSource, Moqui's getConnection()
bypasses enlistConnection() entirely and calls ds.getConnection() directly, returning a
ConnectionJavaProxy. When XA START is rejected (XAER_RMFAIL), the connection is never
enrolled in the TX, but Bitronix's release() still requeues it to the pool with the dirty
XA branch intact. Every subsequent checkout of that connection fails the same way,
producing a cascade XAER_RMFAIL errors.

Fix — ContextJavaUtil.java (ConnectionWrapper):
- Added volatile destroyOnClose flag.
- Added isXaRmFail() that traverses the exception cause chain checking for
  XAException.XAER_RMFAIL (JTA spec constant -7). Avoids fragile string matching on
  exception messages which are locale/driver-version sensitive.
- Wrapped createStatement(), prepareStatement(String), and prepareCall(String) with
  try-catch: sets destroyOnClose = true on XAER_RMFAIL and rethrows. Only the three base
  variants are wrapped because Bitronix auto-enlistment fires once on the first statement
  call, and EntityQueryBuilder exclusively uses prepareStatement(String sql).
- closeInternal() calls physicallyDestroy() when destroyOnClose is true.
  physicallyDestroy() uses reflection to invoke JdbcPooledConnection.close() via
  ConnectionJavaProxy.getPooledConnection(), which unregisters the connection from the
  Bitronix pool and closes the physical DB connection, preventing the dirty XA branch
  from being recycled. Falls back to con.close() if reflection fails (non-Bitronix pools).

Fix — TransactionFacadeImpl.groovy:
- suspend(): removed closeTxConnections() before tm.suspend(). Stashed connections belong
  to the outer TX's suspended XA branches and must not be returned to pool mid-suspend;
  they are released when the outer TX resumes and eventually commits or rolls back.
- enlistConnection(): added finally { if (con.close() } to destroy the
  XAConnection on enlistment or connection-retrieval failure for raw XADataSource paths
  (non-Bitronix), preventing the same dirty-connection cascade on those configurations.
@dixitdeepak

dixitdeepak commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

When MySQL marks an XA branch as rollback-only (e.g. after a deadlock or timeout),
Bitronix auto-enlistment fails with XAER_RMFAIL on the next statement call. Because
PoolingDataSource does not implement javax.sql.XADataSource, Moqui's getConnection()
bypasses enlistConnection() entirely and calls ds.getConnection() directly, returning a
ConnectionJavaProxy. When XA START is rejected (XAER_RMFAIL), the connection is never
enrolled in the TX, but Bitronix's release() still requeues it to the pool with the dirty
XA branch intact. Every subsequent checkout of that connection fails the same way,
producing a cascade XAER_RMFAIL errors

Fix — ContextJavaUtil.java (ConnectionWrapper):

  • Added volatile destroyOnClose flag.
  • Added isXaRmFail() that traverses the exception cause chain checking for
    XAException.XAER_RMFAIL (JTA spec constant -7). Avoids fragile string matching on
    exception messages which are locale/driver-version sensitive.
  • Wrapped createStatement(), prepareStatement(String), and prepareCall(String) with
    try-catch: sets destroyOnClose = true on XAER_RMFAIL and rethrows. Only the three base
    variants are wrapped because Bitronix auto-enlistment fires once on the first statement
    call, and EntityQueryBuilder exclusively uses prepareStatement(String sql).
  • closeInternal() calls physicallyDestroy() when destroyOnClose is true.
    physicallyDestroy() uses reflection to invoke JdbcPooledConnection.close() via
    ConnectionJavaProxy.getPooledConnection(), which unregisters the connection from the
    Bitronix pool and closes the physical DB connection, preventing the dirty XA branch
    from being recycled. Falls back to con.close() if reflection fails (non-Bitronix pools).

Fix — TransactionFacadeImpl.groovy:

  • suspend(): removed closeTxConnections() before tm.suspend(). Stashed connections belong
    to the outer TX's suspended XA branches and must not be returned to pool mid-suspend;
    they are released when the outer TX resumes and eventually commits or rolls back.
  • enlistConnection(): added finally { if (!enlisted) con.close() } to destroy the
    XAConnection on enlistment or connection-retrieval failure for raw XADataSource paths
    (non-Bitronix), preventing the same dirty-connection cascade on those configurations.

@dixitdeepak
dixitdeepak marked this pull request as draft August 5, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant