From f165f954dd5d95e00d87a5c2c033caacf2f0930e Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Mon, 14 Jan 2013 13:14:47 +0000
Subject: [PATCH] SP-387, BIS-255: bug in case of three threads fixed and
 tested.

SVN: 28080
---
 .../generic/shared/content/ContentCache.java  | 23 ++++--
 ...ierarchicalContentNodeMultiThreadTest.java | 81 +++++++++++++++++++
 2 files changed, 96 insertions(+), 8 deletions(-)

diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/ContentCache.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/ContentCache.java
index e74d9f438fb..83fd664fcc9 100644
--- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/ContentCache.java
+++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/ContentCache.java
@@ -26,6 +26,7 @@ import java.net.URL;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.commons.io.IOUtils;
@@ -245,34 +246,40 @@ public class ContentCache implements IContentCache
             }
         }
     }
-
     
     private static final class LockManager
     {
-        private final Map<String, ReentrantLock> locks = new HashMap<String, ReentrantLock>();
+        private static final class LockWithCounter
+        {
+            private Lock lock = new ReentrantLock();
+            private int count;
+        }
+        
+        private final Map<String, LockWithCounter> locks = new HashMap<String, LockWithCounter>();
 
         void lock(String path)
         {
-            ReentrantLock lock;
+            LockWithCounter lock;
             synchronized (locks)
             {
                 lock = locks.get(path);
                 if (lock == null)
                 {
-                    lock = new ReentrantLock();
+                    lock = new LockWithCounter();
                     locks.put(path, lock);
                 }
+                lock.count++;
             }
-            lock.lock();
+            lock.lock.lock();
         }
 
         synchronized void unlock(String path)
         {
-            ReentrantLock lock = locks.get(path);
+            LockWithCounter lock = locks.get(path);
             if (lock != null)
             {
-                lock.unlock();
-                if (lock.isLocked() == false)
+                lock.lock.unlock();
+                if (--lock.count == 0)
                 {
                     locks.remove(path);
                 }
diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/RemoteHierarchicalContentNodeMultiThreadTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/RemoteHierarchicalContentNodeMultiThreadTest.java
index 5826fcc637f..7e2187e6f81 100644
--- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/RemoteHierarchicalContentNodeMultiThreadTest.java
+++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/content/RemoteHierarchicalContentNodeMultiThreadTest.java
@@ -259,6 +259,87 @@ public class RemoteHierarchicalContentNodeMultiThreadTest extends AbstractRemote
         context.assertIsSatisfied();
     }
 
+    
+    @Test
+    public void testGetSameFileInThreeThreads() throws Exception
+    {
+        ContentCache cache = createCache(false);
+        final DataSetPathInfo pathInfo = new DataSetPathInfo();
+        pathInfo.setRelativePath(remoteFile1.getName());
+        pathInfo.setDirectory(false);
+        final IHierarchicalContentNode node1 = createRemoteNode(pathInfo, cache);
+        ConsoleLogger logger = new ConsoleLogger();
+        final MessageChannel channel1 =
+                new MessageChannelBuilder(10000).name("1").logger(logger).getChannel();
+        final IHierarchicalContentNode node2 = createRemoteNode(pathInfo, cache);
+        final MessageChannel channel2 =
+                new MessageChannelBuilder(10000).name("2").logger(logger).getChannel();
+        final IHierarchicalContentNode node3 = createRemoteNode(pathInfo, cache);
+        final MessageChannel channel3 =
+                new MessageChannelBuilder(10000).name("3").logger(logger).getChannel();
+        final MessageChannel channel4 =
+                new MessageChannelBuilder(10000).name("4").logger(logger).getChannel();
+        GetFileRunnable fileRunnable1 = new GetFileRunnable(node1, channel1);
+        GetFileRunnable fileRunnable2 = new GetFileRunnable(node2, channel2)
+        {
+            @Override
+            public void run()
+            {
+                channel1.assertNextMessage(STARTED_MESSAGE);
+                channel2.send(STARTED_MESSAGE);
+                super.run();
+            }
+        };
+        GetFileRunnable fileRunnable3 = new GetFileRunnable(node3, channel3)
+        {
+            @Override
+            public void run()
+            {
+                channel2.assertNextMessage(STARTED_MESSAGE);
+                channel3.send(STARTED_MESSAGE);
+                super.run();
+            }
+        };
+        final Thread thread1 = new Thread(fileRunnable1, "thread1");
+        final Thread thread2 = new Thread(fileRunnable2, "thread2");
+        final Thread thread3 = new Thread(fileRunnable3, "thread3");
+        context.checking(new Expectations()
+        {
+            {
+                one(remoteDss).getDownloadUrlForFileForDataSet(SESSION_TOKEN, DATA_SET_CODE,
+                        pathInfo.getRelativePath());
+                will(new ProxyAction(returnValue(remoteFile1.toURI().toURL().toString()))
+                {
+                    @Override
+                    protected void doBeforeReturn()
+                    {
+                        channel1.send(STARTED_MESSAGE);
+                        channel3.assertNextMessage(STARTED_MESSAGE);
+                        channel4.send(FINISHED_MESSAGE);
+                    }
+                });
+            }
+        });
+        
+        thread1.start();
+        thread2.start();
+        thread3.start();
+        channel4.assertNextMessage(FINISHED_MESSAGE);
+        channel1.assertNextMessage(FINISHED_MESSAGE);
+        channel2.assertNextMessage(FINISHED_MESSAGE);
+        channel3.assertNextMessage(FINISHED_MESSAGE);
+        
+        File file1 = fileRunnable1.tryGetResult();
+        File file2 = fileRunnable2.tryGetResult();
+        File file3 = fileRunnable3.tryGetResult();
+        assertEquals(new File(workSpace, ContentCache.CACHE_FOLDER + "/" + DATA_SET_CODE + "/"
+                + remoteFile1.getName()).getAbsolutePath(), file1.getAbsolutePath());
+        assertEquals(FILE1_CONTENT, FileUtilities.loadToString(file1).trim());
+        assertEquals(file1, file2);
+        assertEquals(file1, file3);
+        context.assertIsSatisfied();
+    }
+    
     @Test
     public void testGetSameFileInTwoThreadsFirstDownloadFails() throws Exception
     {
-- 
GitLab