ALF-12866: WebDAV in-memory locking - threading issues.

* LockInfo instances provide a ReentrantReadWriteLock for clients to use
* LockInfo client code synchronises uses provided RRWLs.



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

source/java/org/alfresco/repo/webdav/LockInfo.java

@@ -22,9 +22,13 @@ import java.io.Serializable;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
- * Class to represent a WebDAV lock info
+ * Class to represent a WebDAV lock info. Instances of this class are accessible
+ * my multiple threads as they are kept in the {@link LockStore}. Clients of this
+ * class are expected to synchronise externally using the provided
+ * ReentrantReadWriteLock (use {@link #getRWLock()}).
  * 
  * @author Ivan Rybnikov
  *
@@ -33,6 +37,8 @@ public final class LockInfo implements Serializable
 {
     private static final long serialVersionUID = 1L;
 
+    private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
+    
     // Exclusive lock token
     private String exclusiveLockToken = null;
 
@@ -73,6 +79,20 @@ public final class LockInfo implements Serializable
         this.depth = depth;
     }
 
+    /**
+     * Retrieves the {@link ReentrantReadWriteLock} associated with this LockInfo. This is
+     * to allow client code to protect against invalid concurrent access to the state of
+     * this class.
+     * <p>
+     * Not to be confused with WebDAV locks.
+     * 
+     * @return
+     */
+    public ReentrantReadWriteLock getRWLock()
+    {
+        return rwLock;
+    }
+    
     /**
      * Returns true if node has shared or exclusive locks
      * 
@@ -294,15 +314,23 @@ public final class LockInfo implements Serializable
     }
 
     /**
-     * Sets the expiry date/time to lockTimeout seconds into the future.
+     * Sets the expiry date/time to lockTimeout seconds into the future. Provide
+     * a lockTimeout of WebDAV.TIMEOUT_INFINITY for never expires.
      * 
      * @param lockTimeout
      */
     public void setTimeoutSeconds(int lockTimeout)
+    {
+        if (lockTimeout == WebDAV.TIMEOUT_INFINITY)
+        {
+            setExpires(null);
+        }
+        else
         {
             int timeoutMillis = (lockTimeout * 60 * 1000);
             Date now = new Date();
             Date nextExpiry = new Date(now.getTime() + timeoutMillis);
             setExpires(nextExpiry);
         }
+    }
 }

source/java/org/alfresco/repo/webdav/LockMethod.java

@@ -336,12 +336,13 @@ public class LockMethod extends WebDAVMethod
             m_response.setStatus(HttpServletResponse.SC_CREATED);
         }
 
-
-        
         // Check if this is a new lock or a lock refresh
         if (hasLockToken())
         {
-            this.lockInfo = checkNode(lockNodeInfo);
+            lockInfo = checkNode(lockNodeInfo);
+            lockInfo.getRWLock().writeLock().lock();
+            try
+            {
                 // If a request body is not defined and "If" header is sent we have createExclusive as false,
                 // but we need to check a previous LOCK was an exclusive. I.e. get the property for node. It
                 // is already has got in a checkNode method, so we need just get a scope from lockInfo.
@@ -350,12 +351,25 @@ public class LockMethod extends WebDAVMethod
                 // Refresh an existing lock
                 refreshLock(lockNodeInfo, userName);
             }
+            finally
+            {
+                lockInfo.getRWLock().writeLock().unlock();
+            }
+        }
         else
         {
-            this.lockInfo = checkNode(lockNodeInfo, true, this.createExclusive);
+            lockInfo = checkNode(lockNodeInfo, true, createExclusive);
+            lockInfo.getRWLock().writeLock().lock();
+            try
+            {
                 // Create a new lock
                 createLock(lockNodeInfo, userName);
             }
+            finally
+            {
+                lockInfo.getRWLock().writeLock().unlock();
+            }
+        }
 
 
         m_response.setHeader(WebDAV.HEADER_LOCK_TOKEN, "<" + WebDAV.makeLockToken(lockNodeInfo.getNodeRef(), userName) + ">");
@@ -391,6 +405,9 @@ public class LockMethod extends WebDAVMethod
         // Create Lock token
         lockToken = WebDAV.makeLockToken(lockNode.getNodeRef(), userName);
 
+        lockInfo.getRWLock().writeLock().lock();
+        try
+        {
             if (createExclusive)
             {
                 // Lock the node
@@ -417,6 +434,11 @@ public class LockMethod extends WebDAVMethod
                 logger.debug("Locked node " + lockNode + ": " + lockInfo);
             }
         }
+        finally
+        {
+            lockInfo.getRWLock().writeLock().unlock();
+        }
+    }
 
     /**
      * Refresh an existing lock
@@ -429,11 +451,16 @@ public class LockMethod extends WebDAVMethod
     {
         if (this.createExclusive)
         {
-            // Update the expiry for the lock
+            lockInfo.getRWLock().writeLock().lock();
-            if (lockInfo.getExpires() != null)
+            try
             {
+                // Update the expiry for the lock
                 lockInfo.setTimeoutSeconds(getLockTimeout());
             }
+            finally
+            {
+                lockInfo.getRWLock().writeLock().unlock();
+            }
         }
     }
 
@@ -444,6 +471,12 @@ public class LockMethod extends WebDAVMethod
     {
         String scope;
         String lt;
+        String owner;
+        Date expiry;
+        
+        lockInfo.getRWLock().readLock().lock();
+        try
+        {
             if (lockToken != null)
             {
                 // In case of lock creation take the scope from request header
@@ -458,7 +491,13 @@ public class LockMethod extends WebDAVMethod
                 // Output refreshed lock
                 lt = this.lockInfo.getExclusiveLockToken();
             }
-        String owner = lockInfo.getOwner();
+            owner = lockInfo.getOwner();
+            expiry = lockInfo.getExpires();
+        }
+        finally
+        {
+            lockInfo.getRWLock().readLock().unlock();
+        }
         
         XMLWriter xml = createXMLWriter();
 
@@ -468,7 +507,6 @@ public class LockMethod extends WebDAVMethod
         xml.startElement(WebDAV.DAV_NS, WebDAV.XML_PROP + nsdec, WebDAV.XML_NS_PROP + nsdec, getDAVHelper().getNullAttributes());
 
         // Output the lock details
-        Date expiry = lockInfo.getExpires();
         generateLockDiscoveryXML(xml, lockNodeInfo, false, scope, WebDAV.getDepthName(m_depth), lt, owner, expiry);
 
         // Close off the XML

source/java/org/alfresco/repo/webdav/LockStore.java

@@ -54,5 +54,11 @@ public interface LockStore
     LockInfo get(NodeRef nodeRef);
     
     
+    /**
+     * Remove LockInfo for the specified NodeRef. The LockInfo cannot be considered locked
+     * once removed from the LockStore.
+     * 
+     * @param nodeRef
+     */
     void remove(NodeRef nodeRef);
 }

source/java/org/alfresco/repo/webdav/PropFindMethod.java

@@ -924,11 +924,19 @@ public class PropFindMethod extends WebDAVMethod
         // Output the lock status response
 
         LockInfo lockInfo = getNodeLockInfo(nodeInfo);
+        lockInfo.getRWLock().readLock().lock();
+        try
+        {
             if (lockInfo.isLocked())
             {
                 generateLockDiscoveryXML(xml, nodeInfo, lockInfo);
             }
         }
+        finally
+        {            
+            lockInfo.getRWLock().readLock().unlock();
+        }
+    }
 
     /**
      * Output the supported lock types XML element

source/java/org/alfresco/repo/webdav/PutMethod.java

@@ -190,7 +190,12 @@ public class PutMethod extends WebDAVMethod
         String userName = getDAVHelper().getAuthenticationService().getCurrentUserName();
         LockInfo lockInfo = getLockStore().get(contentNodeInfo.getNodeRef());
         
-        if (lockInfo != null && lockInfo.isLocked() && !lockInfo.getOwner().equals(userName))
+        if (lockInfo != null)
+        {
+            lockInfo.getRWLock().readLock().lock();
+            try
+            {
+                if (lockInfo.isLocked() && !lockInfo.getOwner().equals(userName))
                 {
                     if (logger.isDebugEnabled())
                     {
@@ -201,6 +206,12 @@ public class PutMethod extends WebDAVMethod
                     // Indicate that the resource is locked
                     throw new WebDAVServerException(WebDAV.WEBDAV_SC_LOCKED);
                 }
+            }
+            finally
+            {
+                lockInfo.getRWLock().readLock().unlock();            
+            }
+        }
         
         try
         {

source/java/org/alfresco/repo/webdav/UnlockMethod.java

@@ -132,7 +132,20 @@ public class UnlockMethod extends WebDAVMethod
         NodeRef nodeRef = lockNodeInfo.getNodeRef();        
         LockInfo lockInfo = getLockStore().get(nodeRef);
         
-        if (lockInfo == null || !lockInfo.isLocked())
+        if (lockInfo == null)
+        {
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("Unlock token=" + getLockToken() + " Not locked - no info in lock store.");
+            }
+            // Node is not locked
+            throw new WebDAVServerException(HttpServletResponse.SC_PRECONDITION_FAILED);
+        }
+        
+        lockInfo.getRWLock().writeLock().lock();
+        try
+        {
+            if (!lockInfo.isLocked())
             {
                 if (logger.isDebugEnabled())
                 {
@@ -200,6 +213,11 @@ public class UnlockMethod extends WebDAVMethod
                 throw new IllegalStateException("Invalid LockInfo state: " + lockInfo);
             }
         }
+        finally
+        {
+            lockInfo.getRWLock().writeLock().unlock();            
+        }
+    }
 
     // This method removes a new zero byte node that has been locked, where the
     // PUT has not taken place but the client has issued an UNLOCK. The Timer

source/java/org/alfresco/repo/webdav/WebDAVMethod.java

@@ -751,9 +751,23 @@ public abstract class WebDAVMethod
      */
     protected void generateLockDiscoveryXML(XMLWriter xml, FileInfo lockNodeInfo, LockInfo lockInfo) throws Exception
     {
-        String owner = lockInfo.getOwner();
+        String owner, scope, depth;
-        Date expiry = lockInfo.getExpires();
+        Date expiry;
-	    generateLockDiscoveryXML(xml, lockNodeInfo, false, lockInfo.getScope(), lockInfo.getDepth(),
+        
+        lockInfo.getRWLock().readLock().lock();
+        try
+        {
+            owner = lockInfo.getOwner();
+            expiry = lockInfo.getExpires();
+    	    scope = lockInfo.getScope();
+            depth = lockInfo.getDepth();
+        }
+        finally
+        {
+            lockInfo.getRWLock().readLock().unlock();            
+        }
+        
+        generateLockDiscoveryXML(xml, lockNodeInfo, false, scope, depth,
 	                WebDAV.makeLockToken(lockNodeInfo.getNodeRef(), owner), owner, expiry);
 	}
   
@@ -873,6 +887,10 @@ public abstract class WebDAVMethod
     protected LockInfo checkNode(FileInfo fileInfo, boolean ignoreShared, boolean lockMethod) throws WebDAVServerException
     {
         LockInfo nodeLockInfo = getNodeLockInfo(fileInfo);
+        
+        nodeLockInfo.getRWLock().readLock().lock();
+        try
+        {
             String nodeETag = getDAVHelper().makeQuotedETag(fileInfo);
 
             
@@ -928,6 +946,11 @@ public abstract class WebDAVMethod
             
             return nodeLockInfo;
         }
+        finally
+        {
+            nodeLockInfo.getRWLock().readLock().unlock();            
+        }
+    }
 
     /**
      * Checks if write operation can be performed on node.
@@ -945,13 +968,15 @@ public abstract class WebDAVMethod
     /**
      * Checks if node can be accessed with WebDAV operation
      * 
-     * @param nodeLockToken      - token to check
      * @param lockInfo           - node's lock info
      * @param ignoreShared       - if true - ignores shared lock tokens 
      * @param lockMethod         - must be true if used from lock method
      * @throws WebDAVServerException if node has no appropriate lock token
      */
     private void checkLockToken(LockInfo lockInfo, boolean ignoreShared, boolean lockMethod) throws WebDAVServerException
+    {
+        lockInfo.getRWLock().readLock().lock();
+        try
         {
             String nodeLockToken = lockInfo.getExclusiveLockToken();
             Set<String> sharedLockTokens = lockInfo.getSharedLockTokens();
@@ -1013,6 +1038,11 @@ public abstract class WebDAVMethod
                     throw new WebDAVServerException(WebDAV.WEBDAV_SC_LOCKED);
                 }
             }
+        }
+        finally
+        {
+            lockInfo.getRWLock().readLock().unlock();            
+        }
                 
         throw new WebDAVServerException(WebDAV.WEBDAV_SC_LOCKED);
     }
@@ -1136,10 +1166,21 @@ public abstract class WebDAVMethod
             
             lockInfo = m_parentLockInfo.get(parent);
             
-            if ((lockInfo != null) && (lockInfo.isLocked()))
+            if (lockInfo != null)
+            {
+                lockInfo.getRWLock().readLock().lock();
+                try
+                {
+                    if (lockInfo.isLocked())
                     {
                         return lockInfo;
                     }
+                }
+                finally
+                {
+                    lockInfo.getRWLock().readLock().unlock();                    
+                }
+            }
             
             if (lockInfo == null)
             {
@@ -1177,13 +1218,25 @@ public abstract class WebDAVMethod
     {
         LockInfo lock = getLockStore().get(nodeInfo.getNodeRef());
         
-        if (lock != null && lock.isLocked())
+        if (lock == null)
+        {
+            return null;
+        }
+        
+        lock.getRWLock().readLock().lock();
+        try
+        {
+            if (lock.isLocked())
             {
                 return lock;
             }
-        
             return null;
         }
+        finally
+        {            
+            lock.getRWLock().readLock().unlock();
+        }        
+    }
     
     /**
      * Checks whether a parent node has a lock that is valid for all its descendants.
@@ -1195,18 +1248,27 @@ public abstract class WebDAVMethod
     {
         LockInfo parentLock = getLockStore().get(parent);
         
-        if (parentLock != null && WebDAV.INFINITY.equals(parentLock.getDepth()))
+        if (parentLock == null)
+        {
+            return null;
+        }
+
+        parentLock.getRWLock().readLock().lock();
+        try
         {
             // now check for lock status ...
-            if (parentLock.isLocked()) 
+            if (parentLock.isLocked() && WebDAV.INFINITY.equals(parentLock.getDepth()))
             {
                 // In this case node is locked indirectly.
                 return parentLock;
             }
-        }
-        
             return null;
         }
+        finally
+        {
+            parentLock.getRWLock().readLock().unlock();
+        }
+    }
     
     /**
      * Get the file info for the given paths