From 44d2fc6bec3f03062aae52021d9700a39f208c2c Mon Sep 17 00:00:00 2001 From: Derek Hulley Date: Thu, 25 Feb 2010 23:46:34 +0000 Subject: [PATCH] Fixed ALF-1562: AuditModelRegistry and audit loading can fail when started on demand - Transactional caches in nested transactions force the outer txn caches to drop new data - Forces a requery of transactional data in case the inner transaction caused it to go stale git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@18868 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org/alfresco/repo/cache/CacheTest.java | 37 ++++++++- .../repo/cache/TransactionalCache.java | 77 ++++++++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/source/java/org/alfresco/repo/cache/CacheTest.java b/source/java/org/alfresco/repo/cache/CacheTest.java index 70ed1bf488..dd0e39073b 100644 --- a/source/java/org/alfresco/repo/cache/CacheTest.java +++ b/source/java/org/alfresco/repo/cache/CacheTest.java @@ -163,6 +163,41 @@ public class CacheTest extends TestCase txnHelper.doInTransaction(callback); } + /** + * Make sure that an outer transaction gets to see cached data modified by an inner transaction. + */ + public void testOuterTransactionStaleDataAvoidance() throws Throwable + { + final TransactionService transactionService = serviceRegistry.getTransactionService(); + + RetryingTransactionCallback outer = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + // Put a value in the cache + transactionalCache.put("A", "OUTER"); + assertNotNull("Expected to get a value before inner txn.", transactionalCache.get("A")); + + RetryingTransactionCallback inner = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + // Just update the cache + transactionalCache.put("A", "INNER"); + return null; + } + }; + // Do it in a nested txn + transactionService.getRetryingTransactionHelper().doInTransaction(inner, false, true); + + assertNull("Expected to *not* get a value after inner txn.", transactionalCache.get("A")); + + return null; + } + }; + transactionService.getRetryingTransactionHelper().doInTransaction(outer); + } + public void testTransactionalCacheWithSingleTxn() throws Throwable { String newGlobalOne = "new_global_one"; @@ -345,7 +380,7 @@ public class CacheTest extends TestCase } /** - * Tests a straight Ehcache adapter against a transactional cache both in and out + * Tests a straight Ehcache adapter against a transactional cache both in and outprojects/repository/source/java/org/alfresco/repo/cache/TransactionalCache.java * of a transaction. This is done repeatedly, pushing the count up. */ public void testPerformance() throws Exception diff --git a/source/java/org/alfresco/repo/cache/TransactionalCache.java b/source/java/org/alfresco/repo/cache/TransactionalCache.java index 41999b6229..14600d0262 100644 --- a/source/java/org/alfresco/repo/cache/TransactionalCache.java +++ b/source/java/org/alfresco/repo/cache/TransactionalCache.java @@ -28,6 +28,7 @@ import java.io.Serializable; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Set; import net.sf.ehcache.Cache; import net.sf.ehcache.CacheException; @@ -97,6 +98,9 @@ public class TransactionalCache /** a unique string identifying this instance when binding resources */ private String resourceKeyTxnData; + + /** Keep track of what transactions have been up to before */ + private ThreadLocal> threadTxnData; /** * Public constructor. @@ -105,6 +109,7 @@ public class TransactionalCache { logger = LogFactory.getLog(TransactionalCache.class); isDebugEnabled = logger.isDebugEnabled(); + threadTxnData = new ThreadLocal>(); } /** @@ -208,7 +213,7 @@ public class TransactionalCache if (data == null) { String txnId = AlfrescoTransactionSupport.getTransactionId(); - data = new TransactionData(); + data = new TransactionData(txnId); // create and initialize caches data.updatedItemsCache = new Cache( name + "_"+ txnId + "_updates", @@ -216,6 +221,7 @@ public class TransactionalCache data.removedItemsCache = new Cache( name + "_" + txnId + "_removes", maxCacheSize, false, true, 0, 0); + try { cacheManager.addCache(data.updatedItemsCache); @@ -232,6 +238,15 @@ public class TransactionalCache AlfrescoTransactionSupport.bindListener(this); } AlfrescoTransactionSupport.bindResource(resourceKeyTxnData, data); + + // Put the data into the set for potential inner transaction usage + Set threadSet = threadTxnData.get(); + if (threadSet == null) + { + threadSet = new HashSet(3); + threadTxnData.set(threadSet); + } + threadSet.add(data); // NB: This is removed during commit or rollback } return data; } @@ -691,6 +706,15 @@ public class TransactionalCache finally { removeCaches(txnData); + + // Remove our txnData entry from the thread + Set threadSet = threadTxnData.get(); + threadSet.remove(txnData); + // We pessimistically kill all new data cached by other transactions + for (TransactionData outerTxnData : threadSet) + { + outerTxnData.removeUpdates(); + } } } @@ -702,6 +726,10 @@ public class TransactionalCache TransactionData txnData = getTransactionData(); // drop caches from cachemanager removeCaches(txnData); + + // Remove our txnData entry from the thread + Set threadSet = threadTxnData.get(); + threadSet.remove(txnData); } /** @@ -858,10 +886,57 @@ public class TransactionalCache /** Data holder to bind data to the transaction */ private class TransactionData { + /** A thread-locally unique ID */ + private String id; + private Cache updatedItemsCache; private Cache removedItemsCache; private boolean haveIssuedFullWarning; private boolean isClearOn; private boolean isClosed; + + private TransactionData(String id) + { + this.id = id; + } + + @Override + public String toString() + { + StringBuilder builder = new StringBuilder(); + builder.append("TransactionData [id=").append(id).append("]"); + return builder.toString(); + } + + @Override + public int hashCode() + { + return id.hashCode(); + } + + /** + * Controlled implementation of equals i.e. we don't check all conditions; the containing + * class needs to ensure types don't get mixed in sets, etc. + */ + @Override + public boolean equals(Object obj) + { + if (this == obj) + { + return true; + } + TransactionData other = (TransactionData) obj; + return id.equals(other.id); + } + + /** + * Pessimistic method that removes all updates and additions from the local + * transactional data. This method is used when inner transactions are started + * that modify the cache. + */ + private void removeUpdates() + { + updatedItemsCache.removeAll(); + } } }