This is an automated email from the ASF dual-hosted git repository.
danhaywood pushed a commit to branch CAUSEWAY-3799
in repository https://gitbox.apache.org/repos/asf/causeway.git
The following commit(s) were added to refs/heads/CAUSEWAY-3799 by this push:
new 647d557a6a CAUSEWAY-3799: adds a guard to attempt to forestall changes
being made
647d557a6a is described below
commit 647d557a6af4192965acb8ae74b5b48ff93e303a
Author: Dan Haywood <[email protected]>
AuthorDate: Fri Jun 28 23:20:47 2024 +0100
CAUSEWAY-3799: adds a guard to attempt to forestall changes being made
once the original transaction is committed
---
.../session/InteractionServiceDefault.java | 32 ++++++++++++++----
.../transaction/TransactionServiceSpring.java | 39 ++++++++++------------
2 files changed, 43 insertions(+), 28 deletions(-)
diff --git
a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/session/InteractionServiceDefault.java
b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/session/InteractionServiceDefault.java
index 79c6d2a935..b7536c6997 100644
---
a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/session/InteractionServiceDefault.java
+++
b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/session/InteractionServiceDefault.java
@@ -363,19 +363,39 @@ implements
@SneakyThrows
private void preInteractionClosed(final CausewayInteraction interaction) {
+ // a bit of a hacky guard
+ //
+ // the suspicion is that if a background command execution encounters
a deadlock then (in
+ // CommandExecutorServiceDefault) the top-level xactn will end up
being rolled back.
+ //
+ // we've seen additional changes being made in a new/implicit (?)
xactn which furthermore are never committed;
+ // we end up with a connection back in Hikari's conn pool with open
locks. If those changes are as a
+ // result of the transaction having completed, then this original
method would have resulted in a command
+ // being persisted
+ //
+ if(transactionServiceSpring.currentTransactionState().isComplete()) {
+ // hmm... someone went and completed (rolled back/committed) our
transaction from us
+ //
+ log.warn("preInteractionClosed: skipping as a precaution because
current transaction has been completed already");
+ return;
+ }
+
Throwable flushException = null;
- try {
- val mustAbort =
transactionServiceSpring.currentTransactionState().mustAbort();
- if(!mustAbort) {
+ val mustAbort =
transactionServiceSpring.currentTransactionState().mustAbort();
+ if(!mustAbort) {
+ try {
transactionServiceSpring.flushTransaction();
// publish only when flush was successful
completeAndPublishCurrentCommand();
+ } catch (Throwable e) {
+ //[CAUSEWAY-3262] if flush fails rethrow later, when
interaction was closed ...
+ flushException = e;
+ transactionServiceSpring.requestRollback(interaction);
}
- } catch (Throwable e) {
- //[CAUSEWAY-3262] if flush fails rethrow later, when interaction
was closed ...
- flushException = e;
}
+ // will either rollback or commit the actual transaction depending
upon whether
+ // anything has called setRollbackOnly so far.
transactionServiceSpring.onClose(interaction);
interactionScopeLifecycleHandler.onTopLevelInteractionPreDestroy(); //
cleanup the InteractionScope (Spring scope)
diff --git
a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/transaction/TransactionServiceSpring.java
b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/transaction/TransactionServiceSpring.java
index 0d5a71fe02..be2bcf3d85 100644
---
a/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/transaction/TransactionServiceSpring.java
+++
b/core/runtimeservices/src/main/java/org/apache/causeway/core/runtimeservices/transaction/TransactionServiceSpring.java
@@ -37,7 +37,6 @@ import org.springframework.stereotype.Service;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.TransactionDefinition;
import org.springframework.transaction.TransactionStatus;
-import org.springframework.transaction.support.DefaultTransactionStatus;
import org.springframework.transaction.support.TransactionSynchronization;
import
org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.transaction.support.TransactionTemplate;
@@ -117,11 +116,9 @@ implements
Try<T> result = null;
try {
-
TransactionStatus txStatus =
platformTransactionManager.getTransaction(def);
registerTransactionSynchronizations(txStatus);
-
result = Try.call(() -> {
final T callResult = callable.call();
@@ -135,22 +132,27 @@ implements
.mapFailure(ex->translateExceptionIfPossible(ex,
platformTransactionManager));
if(result.isFailure()) {
+ // if this is a nested transaction, then the javadoc says it
will actually be just a call to
+ // setRollbackOnly.
platformTransactionManager.rollback(txStatus);
} else {
platformTransactionManager.commit(txStatus);
}
} catch (Exception ex) {
- return result!=null
- && result.isFailure()
-
- // return the original failure cause (originating from
calling the callable)
- // (so we don't shadow the original failure)
- ? result
+ // return the original failure cause (originating from calling the
callable)
+ // (so we don't shadow the original failure)
+ // return the failure we just caught
+ if (result != null && result.isFailure()) {
+ return result;
+ }
- // return the failure we just caught
- : Try.failure(translateExceptionIfPossible(ex,
platformTransactionManager));
+ // otherwise, we thought we had a success, but now we have an
exception thrown by either ,
+ // the call to rollback or commit above. We don't need to do
anything though; if either of
+ // rollback or commit encountered an exception, they will have
implicitly called setRollbackOnly (if nested)
+ // or just rolled-back if top-level.
+ return Try.failure(translateExceptionIfPossible(ex,
platformTransactionManager));
}
return result;
@@ -159,17 +161,10 @@ implements
private void registerTransactionSynchronizations(final TransactionStatus
txStatus) {
if (TransactionSynchronizationManager.isSynchronizationActive()) {
- if (txStatus instanceof DefaultTransactionStatus) {
-
configurableListableBeanFactory.getBeansOfType(TransactionSynchronization.class)
- .values()
- .stream().filter(AopUtils::isAopProxy) // only the
proxies
-
.forEach(TransactionSynchronizationManager::registerSynchronization);
- } else {
-
configurableListableBeanFactory.getBeansOfType(TransactionSynchronization.class)
- .values()
- .stream().filter(AopUtils::isAopProxy) // only the
proxies
-
.forEach(TransactionSynchronizationManager::registerSynchronization);
- }
+
configurableListableBeanFactory.getBeansOfType(TransactionSynchronization.class)
+ .values()
+ .stream().filter(AopUtils::isAopProxy) // only the proxies
+
.forEach(TransactionSynchronizationManager::registerSynchronization);
}
}
