From 6ba121eb0046f84373f27b5dc3e1dbc7405ed2b9 Mon Sep 17 00:00:00 2001
From: Matt Ward <matt.ward@alfresco.com>
Date: Mon, 2 Sep 2013 07:52:11 +0000
Subject: [PATCH] Merged BRANCHES/DEV/HEAD-BUG-FIX to HEAD:    54579:
 ALF-19785: Deadlock during build

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@54739 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
---
 .../repo/lock/mem/AbstractLockStore.java      | 15 +++++++
 .../org/alfresco/repo/lock/mem/LockStore.java |  3 +-
 .../alfresco/repo/lock/mem/LockStoreImpl.java |  6 ++-
 .../RetryingTransactionHelper.java            |  4 +-
 .../lock/mem/AbstractLockStoreTestBase.java   | 44 +++++++++++++++++++
 5 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
index 1ace299d61..d4dacf2b18 100644
--- a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
+++ b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
@@ -30,6 +30,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
  */
 public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockState>> implements LockStore
 {
+    protected long maxTryLockMillis = 100;
     protected T map;
     
     public AbstractLockStore(T map)
@@ -37,6 +38,20 @@ public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockSta
         this.map = map;
     }
     
+    /**
+     * Set the maximum time a lock store should wait while trying to acquire a concurrency lock.
+     * 
+     * @see #acquireConcurrencyLock(NodeRef)
+     * @param maxTryLockMillis
+     */
+    @Override
+    public void setMaxTryLockMillis(long maxTryLockMillis)
+    {
+        this.maxTryLockMillis = maxTryLockMillis;
+    }
+
+
+
     @Override
     public LockState get(NodeRef nodeRef)
     {
diff --git a/source/java/org/alfresco/repo/lock/mem/LockStore.java b/source/java/org/alfresco/repo/lock/mem/LockStore.java
index a487dc2d20..f20fdd3912 100644
--- a/source/java/org/alfresco/repo/lock/mem/LockStore.java
+++ b/source/java/org/alfresco/repo/lock/mem/LockStore.java
@@ -18,10 +18,8 @@
  */
 package org.alfresco.repo.lock.mem;
 
-import java.util.Date;
 import java.util.Set;
 
-import org.alfresco.service.cmr.lock.LockType;
 import org.alfresco.service.cmr.repository.NodeRef;
 
 /**
@@ -58,5 +56,6 @@ public interface LockStore
     void clear();
     void acquireConcurrencyLock(NodeRef nodeRef);
     void releaseConcurrencyLock(NodeRef nodeRef);
+    void setMaxTryLockMillis(long maxTryLockMillis);
     public Set<NodeRef> getNodes();
 }
diff --git a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
index 328c5b41ac..938eaed78e 100644
--- a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
+++ b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
@@ -25,6 +25,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
 
 import org.alfresco.repo.lock.LockServiceImpl;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.util.LockHelper;
 
 import com.google.common.collect.MapMaker;
 
@@ -80,8 +81,9 @@ public class LockStoreImpl extends AbstractLockStore<ConcurrentMap<NodeRef, Lock
     public void acquireConcurrencyLock(NodeRef nodeRef)
     {
         WriteLock writeLock = getWriteLock(nodeRef);
-        // Block until available
-        writeLock.lock();
+        // Block for a short time, if we can't acquire the lock, then throw
+        // an exception that the will allow transaction retry behaviour.
+        LockHelper.tryLock(writeLock, maxTryLockMillis);
     }
 
     @Override
diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java
index 1773a7c7a3..667d5d7208 100644
--- a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java
+++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java
@@ -39,6 +39,7 @@ import org.alfresco.error.ExceptionStackUtil;
 import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState;
 import org.alfresco.service.transaction.TransactionService;
+import org.alfresco.util.LockHelper.LockTryException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.ibatis.exceptions.TooManyResultsException;
@@ -105,7 +106,8 @@ public class RetryingTransactionHelper
                     TooManyResultsException.class,              // Expected one result but found multiple (bad key alert)
                     ObjectNotFoundException.class,
                     CacheException.class,                       // Usually a cache replication issue
-                    SQLGrammarException.class // Actually specific to MS SQL Server 2005 - we check for this
+                    SQLGrammarException.class, // Actually specific to MS SQL Server 2005 - we check for this
+                    LockTryException.class
                     };
      
         List<Class<?>> retryExceptions = new ArrayList<Class<?>>();
diff --git a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
index b136f7e16d..4ad6343c53 100644
--- a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
+++ b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
@@ -21,6 +21,7 @@ package org.alfresco.repo.lock.mem;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.Comparator;
 import java.util.Iterator;
@@ -29,6 +30,7 @@ import java.util.TreeSet;
 
 import org.alfresco.service.cmr.lock.LockType;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.util.LockHelper.LockTryException;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -156,4 +158,46 @@ public abstract class AbstractLockStoreTestBase<T extends LockStore>
         assertEquals(nodeRef3, it.next());
         assertEquals(nodeRef4, it.next());
     }
+    
+    @Test()
+    public void whenTryLockFailsTransactionWillRetry() throws InterruptedException
+    {
+        final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/whenTryLockFailsTransactionWillRetry");
+        
+        // The "other" thread, that got there first and took the lock.
+        Thread thread = new Thread(getClass().getSimpleName()+"-testTryLock")
+        {
+            @Override
+            public void run()
+            {
+                try
+                {
+                    // Deliberately NOT releasing this lock, so we can check for tryLock failure.
+                    lockStore.acquireConcurrencyLock(nodeRef);
+                }
+                catch (Throwable e)
+                {
+                    // This shouldn't happen the first time.
+                    fail("Failed to a acquire lock");
+                }
+            }
+        };
+        thread.start();
+        
+        
+        // Wait for the other thread to perform the lock.
+        thread.join();
+        
+        // Second attempt will fail (already locked above), expected exception
+        // must be thrown for retrying behaviour to work correctly.
+        try
+        {
+            lockStore.acquireConcurrencyLock(nodeRef);
+            fail("Exception was not thrown when unable to acquire lock.");
+        }
+        catch (LockTryException e)
+        {
+            // Good, we got the correct exception.
+        }
+    }
 }