changeset 615:7ea82d063131 Gaya-8.0.x

TW-30589 fix create temp dir race condition
author Dmitry Neverov <dmitry.neverov@gmail.com>
date Thu, 01 Aug 2013 11:53:48 +0400
parents f6efcbc52d11
children 7a4ecffe34a9
files mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtil.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtilTest.java
diffstat 2 files changed, 66 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtil.java	Thu Aug 01 11:35:55 2013 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtil.java	Thu Aug 01 11:53:48 2013 +0400
@@ -8,12 +8,15 @@
 import java.io.File;
 import java.io.FileFilter;
 import java.io.IOException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * @author dmitry.neverov
  */
 public final class HgFileUtil {
 
+  private final static ConcurrentMap<String, Object> myTmpDirLocks = new ConcurrentHashMap<String, Object>();
   private final static String TEMP_DIR_PREFIX = "hg";
 
   private HgFileUtil() {
@@ -28,13 +31,30 @@
     File parentDir = new File(FileUtil.getTempDirectory());
     int suffix = 0;
     File dir;
-    do {
+    while (true) {
       suffix++;
-      dir = new File(parentDir, TEMP_DIR_PREFIX + suffix);
-    } while (dir.exists() || !dir.createNewFile());
-    dir.delete();
-    dir.mkdir();
-    return dir;
+      String tmpDirName = TEMP_DIR_PREFIX + suffix;
+      dir = new File(parentDir, tmpDirName);
+      if (dir.exists())
+        continue;
+
+      synchronized (getTmpDirLock(tmpDirName)) {
+        if (!dir.createNewFile())
+          continue;
+        if (!dir.delete())
+          continue;
+        if (!dir.mkdir())
+          continue;
+      }
+      return dir;
+    }
+  }
+
+
+  private static Object getTmpDirLock(@NotNull String tmpDirName) {
+    Object tmpDirLock = new Object();
+    Object existingLock = myTmpDirLocks.putIfAbsent(tmpDirName, tmpDirLock);
+    return existingLock != null ? existingLock : tmpDirLock;
   }
 
 
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtilTest.java	Thu Aug 01 11:35:55 2013 +0400
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtilTest.java	Thu Aug 01 11:53:48 2013 +0400
@@ -1,11 +1,18 @@
 package jetbrains.buildServer.buildTriggers.vcs.mercurial;
 
 
+import jetbrains.buildServer.util.TestFor;
 import org.testng.annotations.Test;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.CountDownLatch;
 
+import static org.testng.AssertJUnit.assertEquals;
 import static org.testng.AssertJUnit.assertFalse;
 
 @Test
@@ -17,4 +24,37 @@
     assertFalse(tmpDir1.getCanonicalPath().equals(tmpDir2.getCanonicalPath()));
   }
 
+
+  @TestFor(issues = "TW-30589")
+  @Test(invocationCount = 10)
+  public void createTempFile_should_always_return_unique_dir() throws Exception {
+    final CountDownLatch allThreadsInitialized = new CountDownLatch(1);
+    final Set<File> tmpFiles = new HashSet<File>();
+    Runnable createTmpFile = new Runnable() {
+      public void run() {
+        try {
+          allThreadsInitialized.await();
+          tmpFiles.add(HgFileUtil.createTempDir());
+        } catch (Exception e) {
+          //ignore
+        }
+      }
+    };
+
+    List<Thread> threads = new ArrayList<Thread>();
+    for (int i = 0; i < 50; i++) {
+      Thread t = new Thread(createTmpFile);
+      t.start();
+      threads.add(t);
+    }
+
+    allThreadsInitialized.countDown();
+
+    for (Thread t : threads) {
+      t.join();
+    }
+
+    assertEquals("Race condition in createTempDir", threads.size(), tmpFiles.size());
+  }
+
 }