Skip to content

Commit

Permalink
#3423 JtaTransactionManager keeps reference to already closed scope (…
Browse files Browse the repository at this point in the history
…and Transaction/connection ThreadLocal) when JtaTxnListener#afterCompletion is called by a different thread (#3424)

Bugfix for "Long/slow transaction reaper" in Wildfire that can close/rollback a transaction in a different thread. Fix is to use active flag to inactivate the transaction and detect that case in JtaTransactionManager.getCurrentTransaction()
  • Loading branch information
rbygrave authored Jun 20, 2024
1 parent 925ceaf commit d5401ef
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -357,4 +357,8 @@ default int depth() {
*/
void postRollback(Throwable cause);

/**
* Set the transaction to be inactive via external transaction manager.
*/
void deactivateExternal();
}
Original file line number Diff line number Diff line change
Expand Up @@ -447,4 +447,9 @@ public void postCommit() {
public void postRollback(Throwable cause) {
transaction.postRollback(cause);
}

@Override
public void deactivateExternal() {
transaction.deactivateExternal();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,11 @@ public void postRollback(Throwable cause) {
// do nothing
}

@Override
public void deactivateExternal() {
this.active = false;
}

/**
* Return true if the transaction is active.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,11 @@ public boolean isActive() {
return active;
}

@Override
public void deactivateExternal() {
this.active = false;
}

@Override
public final boolean isPersistCascade() {
return persistCascade;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,14 @@ public Object getCurrentTransaction() {
// check current Ebean transaction
SpiTransaction currentEbeanTransaction = scope.inScope();
if (currentEbeanTransaction != null) {
// NOT expecting this so log WARNING
log.log(WARNING, "JTA Transaction - no current txn BUT using current Ebean one {0}", currentEbeanTransaction.id());
return currentEbeanTransaction;
if (currentEbeanTransaction.isActive()) {
// NOT expecting this so log WARNING
log.log(WARNING, "JTA Transaction - no current txn BUT using current Ebean one {0}", currentEbeanTransaction.id());
return currentEbeanTransaction;
} else {
log.log(DEBUG, "JTA Transaction - clearing inActive Ebean transaction {0}", currentEbeanTransaction.id());
scope.clearExternal();
}
}

UserTransaction ut = userTransaction();
Expand Down Expand Up @@ -186,27 +191,27 @@ public void beforeCompletion() {

@Override
public void afterCompletion(int status) {
switch (status) {
case Status.STATUS_COMMITTED:
log.log(DEBUG, "Jta Txn [{0}] committed", transaction.id());
transaction.postCommit();
// Remove this transaction object as it is completed
transactionManager.scope().clearExternal();
break;

case Status.STATUS_ROLLEDBACK:
log.log(DEBUG, "Jta Txn [{0}] rollback", transaction.id());
transaction.postRollback(null);
// Remove this transaction object as it is completed
transactionManager.scope().clearExternal();
break;

default:
log.log(DEBUG, "Jta Txn [{0}] status:{1}", transaction.id(), status);
try {
switch (status) {
case Status.STATUS_COMMITTED:
log.log(DEBUG, "Jta Txn [{0}] committed", transaction.id());
transaction.postCommit();
break;

case Status.STATUS_ROLLEDBACK:
log.log(DEBUG, "Jta Txn [{0}] rollback", transaction.id());
transaction.postRollback(null);
break;

default:
log.log(DEBUG, "Jta Txn [{0}] status:{1}", transaction.id(), status);
}
} finally {
transaction.deactivateExternal();
transactionManager.scope().clearExternal();
// No matter the completion status of the transaction, we release the connection we got from the pool.
JdbcClose.close(transaction.internalConnection());
}

// No matter the completion status of the transaction, we release the connection we got from the pool.
JdbcClose.close(transaction.internalConnection());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public void postRollback(Throwable cause) {
// do nothing
}

@Override
public void deactivateExternal() {
// do nothing
}

@Override
public boolean isLogSql() {
Expand Down

0 comments on commit d5401ef

Please sign in to comment.