diff --git a/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java b/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java index f3020e14be..9b8b19de61 100644 --- a/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java +++ b/source/java/org/alfresco/repo/i18n/MessageServiceImpl.java @@ -56,6 +56,7 @@ import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.service.namespace.RegexQNamePattern; +import org.alfresco.util.LockHelper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.extensions.surf.util.I18NUtil; @@ -172,20 +173,29 @@ public class MessageServiceImpl implements MessageService { String tenantDomain = getTenantDomain(); Set tenantResourceBundleBaseNames = null; - + boolean requireUnlock = true; try { - readLock.lock(); + LockHelper.tryLock(readLock, 100); tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain); } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - readLock.unlock(); + if (requireUnlock) + { + readLock.unlock(); + } } try { - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); if (! tenantResourceBundleBaseNames.contains(resBundlePath)) { @@ -196,9 +206,18 @@ public class MessageServiceImpl implements MessageService clearLoadedResourceBundles(tenantDomain); // force re-load of message cache } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - writeLock.unlock(); + if (requireUnlock) + { + writeLock.unlock(); + } } } @@ -274,10 +293,10 @@ public class MessageServiceImpl implements MessageService Set resourceBundleBaseNamesForAllLocales; String tenantDomain = getTenantDomain(); - + boolean requireUnlock = true; try { - readLock.lock(); + LockHelper.tryLock(readLock, 100); // all locales loadedResourceBundlesForAllLocales = getLoadedResourceBundles(tenantDomain); @@ -286,12 +305,15 @@ public class MessageServiceImpl implements MessageService } finally { - readLock.unlock(); + if (requireUnlock) + { + readLock.unlock(); + } } try { - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); // unload resource bundles for each locale (by tenant, if applicable) if ((loadedResourceBundlesForAllLocales != null) && (cachedMessagesForAllLocales != null)) @@ -364,9 +386,18 @@ public class MessageServiceImpl implements MessageService clearLoadedResourceBundles(tenantDomain); // force re-load of message cache } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - writeLock.unlock(); + if (requireUnlock) + { + writeLock.unlock(); + } } } @@ -391,10 +422,10 @@ public class MessageServiceImpl implements MessageService Map> tenantLoadedResourceBundles = null; Map> tenantCachedMessages = null; Set tenantResourceBundleBaseNames = null; - + boolean requireUnlock = true; try { - readLock.lock(); + LockHelper.tryLock(readLock, 100); tenantLoadedResourceBundles = getLoadedResourceBundles(tenantDomain); loadedBundles = tenantLoadedResourceBundles.get(locale); @@ -405,23 +436,41 @@ public class MessageServiceImpl implements MessageService tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain); loadedBundleCount = tenantResourceBundleBaseNames.size(); } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - readLock.unlock(); + if (requireUnlock) + { + readLock.unlock(); + } } if (loadedBundles == null) { try { - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); loadedBundles = new HashSet(); tenantLoadedResourceBundles.put(locale, loadedBundles); init = true; } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - writeLock.unlock(); + if (requireUnlock) + { + writeLock.unlock(); + } } } @@ -429,15 +478,24 @@ public class MessageServiceImpl implements MessageService { try { - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); props = new HashMap(); tenantCachedMessages.put(locale, props); init = true; } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - writeLock.unlock(); + if (requireUnlock) + { + writeLock.unlock(); + } } } @@ -445,7 +503,7 @@ public class MessageServiceImpl implements MessageService { try { - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); // get registered resource bundles Set resBundleBaseNames = getResourceBundleBaseNames(tenantDomain); @@ -503,9 +561,18 @@ public class MessageServiceImpl implements MessageService logger.info("Message bundles (x " + count + ") loaded for locale " + locale); } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - writeLock.unlock(); + if (requireUnlock) + { + writeLock.unlock(); + } } } @@ -589,14 +656,24 @@ public class MessageServiceImpl implements MessageService public Set getRegisteredBundles() { + boolean requireUnlock = true; try { - readLock.lock(); + LockHelper.tryLock(readLock, 100); return getResourceBundleBaseNames(getTenantDomain()); } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - readLock.unlock(); + if (requireUnlock) + { + readLock.unlock(); + } } } @@ -606,19 +683,37 @@ public class MessageServiceImpl implements MessageService if (resourceBundleBaseNames == null) { + boolean requireUnlock = true; try { // assume caller has read lock - upgrade lock manually readLock.unlock(); - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) - resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain); + resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain); + } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; } finally { - readLock.lock(); // reacquire read without giving up write lock - writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + try + { + LockHelper.tryLock(readLock, 100); // reacquire read without giving up write lock + } + catch (LockHelper.LockTryException lte) + { + // rethrow + throw lte; + } + if (requireUnlock) + { + writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + } } if (resourceBundleBaseNames == null) @@ -651,19 +746,37 @@ public class MessageServiceImpl implements MessageService if (loadedResourceBundles == null) { + boolean requireUnlock = true; try { // assume caller has read lock - upgrade lock manually readLock.unlock(); - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain); } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; + } finally { - readLock.lock(); // reacquire read without giving up write lock - writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + try + { + LockHelper.tryLock(readLock, 100); // reacquire read without giving up write lock + } + catch (LockHelper.LockTryException lte) + { + // rethrow + throw lte; + } + if (requireUnlock) + { + writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + } } if (loadedResourceBundles == null) @@ -704,19 +817,37 @@ public class MessageServiceImpl implements MessageService if (messages == null) { + boolean requireUnlock = true; try { // assume caller has read lock - upgrade lock manually readLock.unlock(); - writeLock.lock(); + LockHelper.tryLock(writeLock, 100); reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster) - messages = messagesCache.get(tenantDomain); + messages = messagesCache.get(tenantDomain); + } + catch (LockHelper.LockTryException lte) + { + requireUnlock = false; + // rethrow + throw lte; } finally { - readLock.lock(); // reacquire read without giving up write lock - writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + try + { + LockHelper.tryLock(readLock, 100); // reacquire read without giving up write lock + } + catch (LockHelper.LockTryException lte) + { + // rethrow + throw lte; + } + if (requireUnlock) + { + writeLock.unlock(); // unlock write, still hold read - caller must unlock the read + } } if (messages == null) diff --git a/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java b/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java index 253887486c..1542c63956 100644 --- a/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/i18n/MessageServiceImplTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2010 Alfresco Software Limited. + * Copyright (C) 2005-2013 Alfresco Software Limited. * * This file is part of Alfresco * @@ -32,6 +32,7 @@ import junit.framework.TestCase; import org.alfresco.model.ContentModel; import org.alfresco.repo.content.MimetypeMap; +import org.alfresco.repo.dictionary.DictionaryDAO; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; @@ -62,15 +63,9 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer { private static ApplicationContext applicationContext = ApplicationContextHelper.getApplicationContext(); - private MessageService messageService; - private NodeService nodeService; - private MutableAuthenticationService authenticationService; - private ContentService contentService; - private static final String BASE_BUNDLE_NAME = "testMessages"; private static final String BASE_RESOURCE_CLASSPATH = "org/alfresco/repo/i18n/"; - private static final String PARAM_VALUE = "television"; private static final String MSG_YES = "msg_yes"; private static final String MSG_NO = "msg_no"; @@ -82,6 +77,14 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer private static final String VALUE_FR_NO = "Non"; private static final String VALUE_FR_PARAMS = "Que non " + PARAM_VALUE + "?"; + private MessageService messageService; + private NodeService nodeService; + private MutableAuthenticationService authenticationService; + private ContentService contentService; + private DictionaryDAO dictionaryDAO; + private TransactionService transactionService; + private AuthenticationComponent authenticationComponent; + /** * Test user details */ @@ -92,10 +95,6 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer */ private StoreRef testStoreRef; - private TransactionService transactionService; - - private AuthenticationComponent authenticationComponent; - private UserTransaction testTX; @@ -114,6 +113,7 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer contentService = (ContentService) applicationContext.getBean("ContentService"); transactionService = (TransactionService) applicationContext.getBean("transactionComponent"); authenticationComponent = (AuthenticationComponent) applicationContext.getBean("authenticationComponent"); + dictionaryDAO = (DictionaryDAO) applicationContext.getBean("dictionaryDAO"); // Re-set the current locale to be the default Locale.setDefault(Locale.ENGLISH); @@ -427,4 +427,50 @@ public class MessageServiceImplTest extends TestCase implements MessageDeployer assertEquals(VALUE_PARAMS, messageService.getMessage(MSG_PARAMS, Locale.getDefault(), new Object[]{PARAM_VALUE})); } + /** + * See MNT-9462 + */ + @SuppressWarnings("deprecation") + public void testDictionaryDAOLock() + { + class DictionaryDAOThread extends Thread + { + @Override + public void run() + { + dictionaryDAO.destroy(); + dictionaryDAO.init(); + } + } + class MessageServiceThread extends Thread + { + @Override + public void run() + { + messageService.destroy(); + messageService.getMessage(MSG_YES); + } + } + DictionaryDAOThread ddt = new DictionaryDAOThread(); + MessageServiceThread mst = new MessageServiceThread(); + ddt.start(); + mst.start(); + try + { + ddt.join(1000); + mst.join(1000); + } + catch (InterruptedException ie) + { + ddt.interrupt(); + mst.interrupt(); + fail("Threads were deadlocked."); + } + if (ddt.isAlive() || mst.isAlive()) + { + ddt.stop(); + mst.stop(); + fail("Threads were deadlocked."); + } + } }