Merged HEAD-BUG-FIX to HEAD (4.2)

55486: Merged V4.1-BUG-FIX (4.1.7) to HEAD-BUG-FIX (4.2)
      54914: Merged DEV to BRANCHES/DEV/V4.1-BUG-FIX:
         54811: Unit test for MNT-9462 : Demonstrate deadlock between DictionaryService and MessageService
         54899: MNT-9462 : WebContainer threads are deadlocked
         Relies on MNT-9517 changes as well


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55775 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

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<String> 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<String> 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<Locale, Set<String>> tenantLoadedResourceBundles = null;
         Map<Locale, Map<String, String>> tenantCachedMessages = null;
         Set<String> 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<String>();
                 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<String, String>();
                 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<String> 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<String> 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
+                try
-                writeLock.unlock(); // unlock write, still hold read - caller must unlock the read
+                {
+                    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
+                try
-                writeLock.unlock(); // unlock write, still hold read - caller must unlock the read
+                {
+                    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
+                try
-                writeLock.unlock(); // unlock write, still hold read - caller must unlock the read
+                {
+                    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)

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.");
+        }
+    }
 }