You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by da...@apache.org on 2016/10/21 15:01:23 UTC
[29/44] isis git commit: ISIS-1502: prevents infinite loop in
IsisTransactionManager#endTransaction() when there is an error in domain
logic code
ISIS-1502: prevents infinite loop in IsisTransactionManager#endTransaction() when there is an error in domain logic code
Project: http://git-wip-us.apache.org/repos/asf/isis/repo
Commit: http://git-wip-us.apache.org/repos/asf/isis/commit/5692abcb
Tree: http://git-wip-us.apache.org/repos/asf/isis/tree/5692abcb
Diff: http://git-wip-us.apache.org/repos/asf/isis/diff/5692abcb
Branch: refs/heads/master
Commit: 5692abcbc0d2c1927cf6ac6cb0c8256e2e342f9f
Parents: d532abc
Author: Dan Haywood <da...@haywood-associates.co.uk>
Authored: Thu Sep 29 10:47:07 2016 +0100
Committer: Dan Haywood <da...@haywood-associates.co.uk>
Committed: Thu Sep 29 10:58:01 2016 +0100
----------------------------------------------------------------------
.../system/transaction/IsisTransaction.java | 2 +-
.../transaction/IsisTransactionManager.java | 116 +++++++++++++------
2 files changed, 80 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/isis/blob/5692abcb/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
index f7eaf60..be7a32a 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransaction.java
@@ -429,7 +429,7 @@ public class IsisTransaction implements TransactionScopedComponent, Transaction
}
- public void commit() {
+ void commit() {
assert getState().canCommit();
assert abortCause == null;
http://git-wip-us.apache.org/repos/asf/isis/blob/5692abcb/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
----------------------------------------------------------------------
diff --git a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
index 4d1f123..560b2a1 100644
--- a/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
+++ b/core/runtime/src/main/java/org/apache/isis/core/runtime/system/transaction/IsisTransactionManager.java
@@ -19,6 +19,7 @@
package org.apache.isis.core.runtime.system.transaction;
+import java.text.MessageFormat;
import java.util.UUID;
import org.slf4j.Logger;
@@ -118,7 +119,7 @@ public class IsisTransactionManager implements SessionScopedComponent {
* {@link IsisTransaction transaction}.
*
* <p>
- * If a transaction is {@link IsisContext#inTransaction() in progress}, then
+ * If a transaction is in progress, then
* uses that. Otherwise will {@link #startTransaction() start} a transaction
* before running the block and {@link #endTransaction() commit} it at the
* end.
@@ -286,22 +287,52 @@ public class IsisTransactionManager implements SessionScopedComponent {
* exception in turn.
*/
public void endTransaction() {
- if (LOG.isDebugEnabled()) {
- LOG.debug("endTransaction: level " + (transactionLevel) + "->" + (transactionLevel - 1));
- }
final IsisTransaction transaction = getCurrentTransaction();
- if (transaction == null || transaction.getState().isComplete()) {
+ if (transaction == null) {
// allow this method to be called >1 with no adverse affects
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("endTransaction: level {} (no transaction exists)", transactionLevel);
+ }
+
return;
+ } else if (transaction.getState().isComplete()) {
+ // allow this method to be called >1 with no adverse affects
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("endTransaction: level {} (previous transaction completed)", transactionLevel);
+ }
+
+ return;
+ } else {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("endTransaction: level {}->{}", transactionLevel, transactionLevel - 1);
+ }
}
+
+ try {
+ endTransactionInternal();
+ } finally {
+ final IsisTransaction.State state = getCurrentTransaction().getState();
+ int transactionLevel = this.transactionLevel;
+ if(transactionLevel == 0 && !state.isComplete()) {
+ LOG.error("endTransaction: inconsistency detected between transactionLevel {} and transactionState '{}'", transactionLevel, state);
+ }
+ }
+ }
+
+ private void endTransactionInternal() {
+
+ final IsisTransaction transaction = getCurrentTransaction();
+
// terminate the transaction early if an abort cause was already set.
RuntimeException abortCause = this.getCurrentTransaction().getAbortCause();
if(transaction.getState().mustAbort()) {
-
+
if (LOG.isDebugEnabled()) {
- LOG.debug("endTransaction: aborting instead [EARLY TERMINATION], abort cause '" + abortCause.getMessage() + "' has been set");
+ LOG.debug("endTransaction: aborting instead [EARLY TERMINATION], abort cause '{}' has been set", abortCause.getMessage());
}
try {
abortTransaction();
@@ -309,12 +340,12 @@ public class IsisTransactionManager implements SessionScopedComponent {
// just in case any different exception was raised...
abortCause = this.getCurrentTransaction().getAbortCause();
} catch(RuntimeException ex) {
-
+
// ... or, capture this most recent exception
abortCause = ex;
}
-
-
+
+
if(abortCause != null) {
// hasn't been rendered lower down the stack, so fall back
throw abortCause;
@@ -324,61 +355,66 @@ public class IsisTransactionManager implements SessionScopedComponent {
}
}
-
- transactionLevel--;
- if (transactionLevel == 0) {
+ // we don't decrement the transactionLevel just yet, because an exception might end up being thrown
+ // (meaning there would be more faffing around to ensure that the transactionLevel
+ // and state of the currentTransaction remain in sync)
+ if ( (transactionLevel - 1) == 0) {
//
// TODO: granted, this is some fairly byzantine coding. but I'm trying to account for different types
// of object store implementations that could start throwing exceptions at any stage.
// once the contract/API for the objectstore is better tied down, hopefully can simplify this...
//
-
+
if(abortCause == null) {
-
+
if (LOG.isDebugEnabled()) {
LOG.debug("endTransaction: committing");
}
try {
getCurrentTransaction().preCommit();
- } catch(RuntimeException ex) {
+ } catch(Exception ex) {
// just in case any new exception was raised...
- abortCause = ex;
- transactionLevel = 1; // because the transactionLevel was decremented earlier
+
+ // this bizarre code because an InvocationTargetException (which is not a RuntimeException) was
+ // being thrown due to a coding error in a domain object
+ abortCause = ex instanceof RuntimeException ? (RuntimeException) ex : new RuntimeException(ex);
+
+ getCurrentTransaction().setAbortCause(new IsisTransactionManagerException(ex));
}
}
-
+
if(abortCause == null) {
try {
persistenceSession.endTransaction();
- } catch(RuntimeException ex) {
+ } catch(Exception ex) {
// just in case any new exception was raised...
- abortCause = ex;
-
+ abortCause = ex instanceof RuntimeException ? (RuntimeException) ex : new RuntimeException(ex);
+
// hacky... moving the transaction back to something other than COMMITTED
- transactionLevel = 1; // because the transactionLevel was decremented earlier
getCurrentTransaction().setAbortCause(new IsisTransactionManagerException(ex));
}
}
- if(abortCause == null) {
- try {
- getCurrentTransaction().commit();
- } catch(RuntimeException ex) {
- // just in case any new exception was raised...
- abortCause = ex;
- transactionLevel = 1; // because the transactionLevel was decremented earlier
- }
- }
+
+ //
+ // ok, everything that could have thrown an exception is now done,
+ // so it's safe to decrement the transaction level
+ //
+ transactionLevel = 0;
// previously we called __isis_endRequest here on all RequestScopedServices. This is now
// done later, in PersistenceSession#close(). If we introduce support for @TransactionScoped
// services, then this would be the place to finalize them.
+
+ //
+ // finally, if an exception was thrown, we rollback the transaction
+ //
if(abortCause != null) {
-
+
if (LOG.isDebugEnabled()) {
LOG.debug("endTransaction: aborting instead, abort cause has been set");
}
@@ -389,17 +425,23 @@ public class IsisTransactionManager implements SessionScopedComponent {
// * we want the existing abortCause to be available
// * the transactionLevel is correctly now at 0.
}
-
+
throw abortCause;
+ } else {
+
+ // keeping things in sync
+ getCurrentTransaction().commit();
}
+
} else if (transactionLevel < 0) {
- LOG.error("endTransaction: transactionLevel=" + transactionLevel);
+ LOG.error("endTransaction: transactionLevel={}", transactionLevel);
transactionLevel = 0;
- throw new IllegalStateException(" no transaction running to end (transactionLevel < 0)");
+ IllegalStateException ex = new IllegalStateException(" no transaction running to end (transactionLevel < 0)");
+ getCurrentTransaction().setAbortCause(new IsisException(ex));
+ throw ex;
}
}
-
public void abortTransaction() {
if (getCurrentTransaction() != null) {
getCurrentTransaction().markAsAborted();