[jbosscache-commits] JBoss Cache SVN: r5688 - in core/trunk/src: main/java/org/jboss/cache/commands and 2 other directories.

Author: manik.surtani(a)jboss.com Date: 2008-04-25 08:11:55 -0400 (Fri, 25 Apr 2008) New Revision: 5688 Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java core/trunk/src/main/java/org/jboss/cache/commands/CacheCommand.java core/trunk/src/main/java/org/jboss/cache/interceptors/InvocationContextInterceptor.java core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java Log: JBCACHE-1327 - prevent leakage of MethodCall and CacheCommand in InvocationContexts Modified: core/trunk/src/main/java/org/jboss/cache/InvocationContext.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2008-04-25 11:19:47 UTC (rev 5687) +++ core/trunk/src/main/java/org/jboss/cache/InvocationContext.java 2008-04-25 12:11:55 UTC (rev 5688) @@ -38,6 +38,7 @@ private boolean txHasMods; private boolean cacheLoaderHasMods; private boolean localRollbackOnly; + @Deprecated private MethodCall methodCall; @Deprecated private CacheCommand executingCommand; @@ -292,6 +293,8 @@ originLocal = true; txHasMods = false; invocationLocks = null; + methodCall = null; + executingCommand = null; } @Override @@ -375,9 +378,10 @@ return methodCall; } - @SuppressWarnings("deprecated") + @SuppressWarnings("deprecation") private MethodCall createMethodCall(MarshallableCommand command) { + if (command == null) return null; MethodCall call = new MethodCall(); call.setMethodId(command.getCommandId()); call.setArgs(command.getParameters()); Modified: core/trunk/src/main/java/org/jboss/cache/commands/CacheCommand.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/commands/CacheCommand.java 2008-04-25 11:19:47 UTC (rev 5687) +++ core/trunk/src/main/java/org/jboss/cache/commands/CacheCommand.java 2008-04-25 12:11:55 UTC (rev 5688) @@ -6,7 +6,6 @@ * @author Mircea.Markus(a)jboss.com * @since 2.2 * todo - reduce dependencies from CacheSPI many commands depend on CacheSPI only for invoceation ctxt. inject ctxt rather than entire CacheSPI - * todo - add toString to all commands */ public interface CacheCommand extends Cloneable { Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/InvocationContextInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/InvocationContextInterceptor.java 2008-04-25 11:19:47 UTC (rev 5687) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/InvocationContextInterceptor.java 2008-04-25 12:11:55 UTC (rev 5688) @@ -111,6 +111,7 @@ return handleAll(ctx, command, null); } + @SuppressWarnings("deprecation") public Object handleAll(InvocationContext ctx, CacheCommand command, GlobalTransaction gtx) throws Throwable { Option optionOverride = ctx.getOptionOverrides(); @@ -186,6 +187,10 @@ copyInvocationScopeOptionsToTxScope(ctx); } } + + // reset the context to prevent leakage of internals + ctx.setExecutingCommand(null); + ctx.setMethodCall(null); } } Modified: core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java =================================================================== --- core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2008-04-25 11:19:47 UTC (rev 5687) +++ core/trunk/src/main/java/org/jboss/cache/interceptors/TxInterceptor.java 2008-04-25 12:11:55 UTC (rev 5688) @@ -738,12 +738,22 @@ * @return * @throws Throwable */ - private Object handleCommitRollback(InvocationContext ctx) throws Throwable + @SuppressWarnings("deprecation") + private Object handleCommitRollback(InvocationContext ctx, CacheCommand command) throws Throwable { - //GlobalTransaction globalTransaction = findGlobalTransaction(m.getArgs()); GlobalTransaction gtx = ctx.getGlobalTransaction(); Object result; - result = invokeNextInterceptor(ctx, ctx.getExecutingCommand()); + CacheCommand originalCommand = ctx.getExecutingCommand(); + ctx.setExecutingCommand(command); + try + { + result = invokeNextInterceptor(ctx, command); + } + finally + { + ctx.setExecutingCommand(originalCommand); + ctx.setMethodCall(null); + } if (log.isDebugEnabled()) log.debug("Finished local commit/rollback method for " + gtx); return result; } @@ -784,8 +794,8 @@ { log.trace(" running commit for " + gtx); } - ctx.setExecutingCommand(commitCommand); - handleCommitRollback(ctx); + + handleCommitRollback(ctx, commitCommand); } catch (Throwable e) { @@ -845,8 +855,7 @@ //ltx = getLocalTxForGlobalTx(globalTransaction); rollbackTransactions.put(tx, gtx); - ctx.setExecutingCommand(rollbackCommand); - handleCommitRollback(ctx); + handleCommitRollback(ctx, rollbackCommand); } catch (Throwable e) { @@ -884,6 +893,7 @@ * @return * @throws Throwable */ + @SuppressWarnings("deprecation") public Object runPreparePhase(InvocationContext ctx, GlobalTransaction gtx, List<TxCacheCommand> modifications) throws Throwable { // build the method call @@ -919,10 +929,19 @@ //if ltx is not null and it is already running if (txManager.getTransaction() != null && ltx != null && txManager.getTransaction().equals(ltx)) { + CacheCommand originalCommand = ctx.getExecutingCommand(); ctx.setExecutingCommand(prepareCommand); // set the hasMods flag in the invocation ctx. This should not be replicated, just used locally by the interceptors. ctx.setTxHasMods(modifications != null && modifications.size() > 0); - result = invokeNextInterceptor(ctx, prepareCommand); + try + { + result = invokeNextInterceptor(ctx, prepareCommand); + } + finally + { + ctx.setExecutingCommand(originalCommand); + ctx.setMethodCall(null); + } } else { Modified: core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java =================================================================== --- core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java 2008-04-25 11:19:47 UTC (rev 5687) +++ core/trunk/src/test/java/org/jboss/cache/invocationcontext/TransactionTest.java 2008-04-25 12:11:55 UTC (rev 5688) @@ -4,7 +4,6 @@ import org.jboss.cache.DefaultCacheFactory; import org.jboss.cache.Fqn; import org.jboss.cache.config.Configuration.CacheMode; - import static org.testng.AssertJUnit.*; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; @@ -90,7 +89,7 @@ public void testScrubbingAfterOnePhaseCommit() throws Exception { setUpOnePhaseCache(); - + doScrubbingTest(true); } @@ -102,15 +101,16 @@ public void testScrubbingAfterOnePhaseRollback() throws Exception { setUpOnePhaseCache(); - + doScrubbingTest(true); } + @SuppressWarnings("deprecation") private void doScrubbingTest(boolean commit) throws Exception { // Start clean cache.getInvocationContext().reset(); - + tm.begin(); cache.getRoot().put("key", "value"); assertNotNull("Tx should have been set up by now", cache.getInvocationContext().getTransaction()); @@ -128,6 +128,8 @@ assertNull("Tx should have been scrubbed", cache.getInvocationContext().getTransaction()); assertNull("Gtx should have been scrubbed", cache.getInvocationContext().getGlobalTransaction()); + assertEquals("Method call should have been scrubbed", null, cache.getInvocationContext().getMethodCall()); + assertEquals("Cache command should have been scrubbed", null, cache.getInvocationContext().getExecutingCommand()); } private void setUpOnePhaseCache() @@ -137,7 +139,7 @@ cache.stop(); cache = null; } - + cache = (CacheSPI<Object, Object>) new DefaultCacheFactory().createCache("META-INF/conf-test/replSync-service.xml", false); cache.getConfiguration().setCacheMode(CacheMode.REPL_ASYNC); cache.start();