Merged HEAD-BUG-FIX to HEAD (4.2)

55487: Merged V4.1-BUG-FIX (4.1.7) to HEAD-BUG-FIX (4.2)
      54917: Follow up to fixes for MNT-9462:
       - Remove pattern where lock-try is in the try-finally block, thereby forcing conditional unlocking
         If the lock is not achieved, then the finally block is not needed so the try-lock can go outside.
       - Fixed lock upgrade to use check, upgrade and recheck; now it's less like the script of Inception


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

source/java/org/alfresco/repo/i18n/MessageServiceImpl.java

@@ -173,30 +173,19 @@ public class MessageServiceImpl implements MessageService
     {
         String tenantDomain = getTenantDomain(); 
         Set<String> tenantResourceBundleBaseNames = null;
-        boolean requireUnlock = true;
+        LockHelper.tryLock(readLock, 100);
         try
         {
-            LockHelper.tryLock(readLock, 100);
             tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain);  
         }
-        catch (LockHelper.LockTryException lte)
-        {
-            requireUnlock = false;
-            // rethrow
-            throw lte;
-        }
         finally
-        {
-            if (requireUnlock)
         {
             readLock.unlock();
         }
-        }
         
+        LockHelper.tryLock(writeLock, 100);
         try
         {
-            LockHelper.tryLock(writeLock, 100);
-            
             if (! tenantResourceBundleBaseNames.contains(resBundlePath))
             {
             	tenantResourceBundleBaseNames.add(resBundlePath);
@@ -206,20 +195,11 @@ public class MessageServiceImpl implements MessageService
             
             clearLoadedResourceBundles(tenantDomain); // force re-load of message cache
         }
-        catch (LockHelper.LockTryException lte)
-        {
-            requireUnlock = false;
-            // rethrow
-            throw lte;
-        }
         finally
-        {
-            if (requireUnlock)
         {
             writeLock.unlock();
         }
     }
-    }
     
     public String getMessage(String messageKey)
     {
@@ -293,28 +273,22 @@ public class MessageServiceImpl implements MessageService
         Set<String> resourceBundleBaseNamesForAllLocales;
         
         String tenantDomain = getTenantDomain();
-        boolean requireUnlock = true;
+        LockHelper.tryLock(readLock, 100);
         try
         {
-            LockHelper.tryLock(readLock, 100);
-
             // all locales
             loadedResourceBundlesForAllLocales = getLoadedResourceBundles(tenantDomain);
             cachedMessagesForAllLocales = getMessages(tenantDomain);
             resourceBundleBaseNamesForAllLocales = getResourceBundleBaseNames(tenantDomain);
         }    
         finally
-        {
-            if (requireUnlock)
         {
             readLock.unlock();
         }
-        }
         
+        LockHelper.tryLock(writeLock, 100);
         try
         {
-            LockHelper.tryLock(writeLock, 100);
-                      
             // unload resource bundles for each locale (by tenant, if applicable)        
             if ((loadedResourceBundlesForAllLocales != null) && (cachedMessagesForAllLocales != null))
             {
@@ -386,20 +360,11 @@ public class MessageServiceImpl implements MessageService
                      
             clearLoadedResourceBundles(tenantDomain); // force re-load of message cache
         }
-        catch (LockHelper.LockTryException lte)
-        {
-            requireUnlock = false;
-            // rethrow
-            throw lte;
-        }
         finally
-        {
-            if (requireUnlock)
         {
             writeLock.unlock();
         }
     }
-    }
     
     /**
      * Get the messages for a locale.
@@ -422,11 +387,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;
+
+        LockHelper.tryLock(readLock, 100);
         try
         {
-            LockHelper.tryLock(readLock, 100);
-
             tenantLoadedResourceBundles = getLoadedResourceBundles(tenantDomain);
             loadedBundles = tenantLoadedResourceBundles.get(locale);
 
@@ -436,75 +400,46 @@ public class MessageServiceImpl implements MessageService
             tenantResourceBundleBaseNames = getResourceBundleBaseNames(tenantDomain);
             loadedBundleCount = tenantResourceBundleBaseNames.size();
         }
-        catch (LockHelper.LockTryException lte)
-        {
-            requireUnlock = false;
-            // rethrow
-            throw lte;
-        }
         finally
-        {
-            if (requireUnlock)
         {
             readLock.unlock();
         }
-        }
 
         if (loadedBundles == null)
         {
+            LockHelper.tryLock(writeLock, 100);
             try
             {
-                LockHelper.tryLock(writeLock, 100);
                 loadedBundles = new HashSet<String>();
                 tenantLoadedResourceBundles.put(locale, loadedBundles);
                 init = true;
             }
-            catch (LockHelper.LockTryException lte)
-            {
-                requireUnlock = false;
-                // rethrow
-                throw lte;
-            }
             finally
-            {
-                if (requireUnlock)
             {
                 writeLock.unlock();
             }
         }
-        }
 
         if (props == null)
         {
+            LockHelper.tryLock(writeLock, 100);
             try
             {
-                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
-            {
-                if (requireUnlock)
             {
                 writeLock.unlock();
             }
         }
-        }
 
         if ((loadedBundles.size() != loadedBundleCount) || (init == true))
         {
+            LockHelper.tryLock(writeLock, 100);
             try
             {
-                LockHelper.tryLock(writeLock, 100);
-
                 // get registered resource bundles               
                 Set<String> resBundleBaseNames = getResourceBundleBaseNames(tenantDomain);
 
@@ -561,20 +496,11 @@ 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
-            {
-                if (requireUnlock)
             {
                 writeLock.unlock();
             }
         }
-        }
 
         return props;
     }
@@ -656,64 +582,43 @@ public class MessageServiceImpl implements MessageService
     
     public Set<String> getRegisteredBundles()
     {
-        boolean requireUnlock = true;
+        LockHelper.tryLock(readLock, 100);
         try
         {
-            LockHelper.tryLock(readLock, 100);
             return getResourceBundleBaseNames(getTenantDomain());
         }
-        catch (LockHelper.LockTryException lte)
-        {
-            requireUnlock = false;
-            // rethrow
-            throw lte;
-        }
         finally
-        {
-            if (requireUnlock)
         {
             readLock.unlock();
         }
     }  
-    }  
     
     private Set<String> getResourceBundleBaseNames(String tenantDomain)
     {
+        // Assume a read lock is present
         Set<String> resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain);
+        if (resourceBundleBaseNames != null)
+        {
+            return resourceBundleBaseNames;
+        }
         
-        if (resourceBundleBaseNames == null)
+        // They are not there.  Upgrade to the write lock.
-        {     
-            boolean requireUnlock = true;
-            try 
-            {
-                // assume caller has read lock - upgrade lock manually
         readLock.unlock();
         LockHelper.tryLock(writeLock, 100);
-                
+        try
+        {
+            resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain);
+            if (resourceBundleBaseNames != null)
+            {
+                return resourceBundleBaseNames;
+            }
             reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster)
             resourceBundleBaseNames = resourceBundleBaseNamesCache.get(tenantDomain);
         }
-            catch (LockHelper.LockTryException lte)
-            {
-                requireUnlock = false;
-                // rethrow
-                throw lte;
-            }
         finally
         {
-                try
+            writeLock.unlock();
-                {
+            LockHelper.tryLock(readLock, 100);
-                    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)
@@ -721,8 +626,7 @@ public class MessageServiceImpl implements MessageService
             // unexpected
             throw new AlfrescoRuntimeException("Failed to re-initialise resourceBundleBaseNamesCache " + tenantDomain);
         }
-        }
+        // Done
-            
         return resourceBundleBaseNames;
     }  
     
@@ -742,41 +646,30 @@ public class MessageServiceImpl implements MessageService
     
     private Map<Locale, Set<String>> getLoadedResourceBundles(String tenantDomain)
     {
+        // Assume a read lock is present
         Map<Locale, Set<String>> loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain);
+        if (loadedResourceBundles != null)
+        {
+            return loadedResourceBundles;
+        }
         
-        if (loadedResourceBundles == null)
+        // Not present.  Upgrade to write lock.
-        {   
-            boolean requireUnlock = true;
-            try 
-            {
-                // assume caller has read lock - upgrade lock manually
         readLock.unlock();
         LockHelper.tryLock(writeLock, 100);
-                
+        try
+        {
+            loadedResourceBundles = loadedResourceBundlesCache.get(tenantDomain);
+            if (loadedResourceBundles != null)
+            {
+                return loadedResourceBundles;
+            }
             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
         {
-                try
+            writeLock.unlock();
-                {
+            LockHelper.tryLock(readLock, 100);
-                    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)
@@ -784,8 +677,7 @@ public class MessageServiceImpl implements MessageService
             // unexpected
             throw new AlfrescoRuntimeException("Failed to re-initialise loadedResourceBundlesCache " + tenantDomain);
         }
-        }
+        // Done
-            
         return loadedResourceBundles;
     }  
     
@@ -813,41 +705,30 @@ public class MessageServiceImpl implements MessageService
     
     private Map<Locale, Map<String, String>> getMessages(String tenantDomain)
     {
+        // Assume a read lock
         Map<Locale, Map<String, String>> messages = messagesCache.get(tenantDomain);
+        if (messages != null)
+        {
+            return messages;
+        }
         
-        if (messages == null)
+        // Need to create it.  Upgrade to write lock.
-        {   
-            boolean requireUnlock = true;
-            try 
-            {
-                // assume caller has read lock - upgrade lock manually
         readLock.unlock();
         LockHelper.tryLock(writeLock, 100);
-                
+        try
+        {
+            messages = messagesCache.get(tenantDomain);
+            if (messages != null)
+            {
+                return messages;
+            }
             reset(tenantDomain); // reset caches - may have been invalidated (e.g. in a cluster)
             messages = messagesCache.get(tenantDomain);
         }
-            catch (LockHelper.LockTryException lte)
-            {
-                requireUnlock = false;
-                // rethrow
-                throw lte;
-            }
         finally
         {
-                try
+            writeLock.unlock();
-                {
+            LockHelper.tryLock(readLock, 100);
-                    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)
@@ -855,8 +736,7 @@ public class MessageServiceImpl implements MessageService
             // unexpected
             throw new AlfrescoRuntimeException("Failed to re-initialise messagesCache " + tenantDomain);
         }
-        }
+        // Done
-            
         return messages;
     }