Author: dereed Date: 2012-06-29 17:04:37 -0400 (Fri, 29 Jun 2012) New Revision: 8507 Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java Log: [JBCACHE-1613] Put fix into a finally block so it works regardless of exceptions/early returns Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2012-06-08 15:56:30 UTC (rev 8506) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2012-06-29 21:04:37 UTC (rev 8507) @@ -903,31 +903,36 @@ // invocationContextContainer.remove themselves protected void beforeCompletion(boolean resetInvocationContext) { - if (trace) log.trace("Running beforeCompletion on gtx " + gtx); - - if (transactionContext == null) + try { - log.error("Transaction has a null transaction entry - beforeCompletion() will fail."); - throw new IllegalStateException("cannot find transaction entry for " + gtx); - } + if (trace) log.trace("Running beforeCompletion on gtx " + gtx); - modifications = transactionContext.getModifications(); - ctx = invocationContextContainer.get(); - setTransactionalContext(tx, gtx, transactionContext, ctx); + if (transactionContext == null) + { + log.error("Transaction has a null transaction entry - beforeCompletion() will fail."); + throw new IllegalStateException("cannot find transaction entry for " + gtx); + } - if (ctx.isOptionsUninitialised() && transactionContext.getOption() != null) - { - ctx.setOptionOverrides(transactionContext.getOption()); - } + modifications = transactionContext.getModifications(); + ctx = invocationContextContainer.get(); + setTransactionalContext(tx, gtx, transactionContext, ctx); - assertCanContinue(); + if (ctx.isOptionsUninitialised() && transactionContext.getOption() != null) + { + ctx.setOptionOverrides(transactionContext.getOption()); + } - ctx.setOriginLocal(false); + assertCanContinue(); - // JBCACHE-1613: remove the ThreadLocal reference to ctx, in case a different - // thread calls afterCompletion - if(resetInvocationContext) - invocationContextContainer.remove(); + ctx.setOriginLocal(false); + } + finally + { + // JBCACHE-1613: remove the ThreadLocal reference to ctx, in case a different + // thread calls afterCompletion + if(resetInvocationContext) + invocationContextContainer.remove(); + } } // this should really not be done here - @@ -1084,79 +1089,84 @@ @Override public void beforeCompletion() { - super.beforeCompletion(false); - ctx.setOriginLocal(!remoteLocal); // this is the LOCAL sync handler after all! - // fetch the modifications before the transaction is committed - // (and thus removed from the txTable) - setTransactionalContext(tx, gtx, transactionContext, ctx); - if (!transactionContext.hasModifications()) + try { - if (trace) log.trace("No modifications in this tx. Skipping beforeCompletion()"); - modifications = Collections.emptyList(); - return; - } - else - { - modifications = transactionContext.getModifications(); - } + super.beforeCompletion(false); + ctx.setOriginLocal(!remoteLocal); // this is the LOCAL sync handler after all! + // fetch the modifications before the transaction is committed + // (and thus removed from the txTable) + setTransactionalContext(tx, gtx, transactionContext, ctx); + if (!transactionContext.hasModifications()) + { + if (trace) log.trace("No modifications in this tx. Skipping beforeCompletion()"); + modifications = Collections.emptyList(); + return; + } + else + { + modifications = transactionContext.getModifications(); + } - // set any transaction wide options as current for this thread, caching original options that would then be reset - originalOptions = ctx.getOptionOverrides(); - transactionalOptions = transactionContext.getOption(); - transactionalOptions.setSuppressEventNotification(originalOptions.isSuppressEventNotification()); - ctx.setOptionOverrides(transactionalOptions); + // set any transaction wide options as current for this thread, caching original options that would then be reset + originalOptions = ctx.getOptionOverrides(); + transactionalOptions = transactionContext.getOption(); + transactionalOptions.setSuppressEventNotification(originalOptions.isSuppressEventNotification()); + ctx.setOptionOverrides(transactionalOptions); - try - { - switch (tx.getStatus()) + try { - // if we are active or preparing then we can go ahead - case Status.STATUS_ACTIVE: - case Status.STATUS_PREPARING: - // run a prepare call. + switch (tx.getStatus()) + { + // if we are active or preparing then we can go ahead + case Status.STATUS_ACTIVE: + case Status.STATUS_PREPARING: + // run a prepare call. - Object result = isOnePhaseCommit() ? null : runPreparePhase(ctx, gtx, modifications); + Object result = isOnePhaseCommit() ? null : runPreparePhase(ctx, gtx, modifications); - if (result instanceof Throwable) - { - if (log.isDebugEnabled()) + if (result instanceof Throwable) { - log.debug("Transaction needs to be rolled back - the cache returned an instance of Throwable for this prepare call (tx=" + tx + " and gtx=" + gtx + ")", (Throwable) result); + if (log.isDebugEnabled()) + { + log.debug("Transaction needs to be rolled back - the cache returned an instance of Throwable for this prepare call (tx=" + tx + " and gtx=" + gtx + ")", (Throwable) result); + } + tx.setRollbackOnly(); + throw (Throwable) result; } - tx.setRollbackOnly(); - throw (Throwable) result; - } - break; - default: - throw new CacheException("transaction " + tx + " in status " + tx.getStatus() + " unable to start transaction"); + break; + default: + throw new CacheException("transaction " + tx + " in status " + tx.getStatus() + " unable to start transaction"); + } } - } - catch (Throwable t) - { - if (log.isWarnEnabled()) log.warn("Caught exception, will now set transaction to roll back", t); - try + catch (Throwable t) { - tx.setRollbackOnly(); + if (log.isWarnEnabled()) log.warn("Caught exception, will now set transaction to roll back", t); + try + { + tx.setRollbackOnly(); + } + catch (SystemException se) + { + throw new RuntimeException("setting tx rollback failed ", se); + } + if (t instanceof RuntimeException) + { + throw (RuntimeException) t; + } + else + { + throw new RuntimeException("", t); + } } - catch (SystemException se) + finally { - throw new RuntimeException("setting tx rollback failed ", se); + localRollbackOnly = false; + setTransactionalContext(null, null, null, ctx); + ctx.setOptionOverrides(originalOptions); } - if (t instanceof RuntimeException) - { - throw (RuntimeException) t; - } - else - { - throw new RuntimeException("", t); - } } finally { - localRollbackOnly = false; - setTransactionalContext(null, null, null, ctx); - ctx.setOptionOverrides(originalOptions); - // JBCACHE-1613: remove the ThreadLocal reference to ctx, in case a different // thread calls afterCompletion invocationContextContainer.remove();