# HG changeset patch # User Dmitry Neverov # Date 1375343628 -14400 # Node ID 7ea82d0631319494173a98da069ae9d2cbecf526 # Parent f6efcbc52d11509b8de3d5b6cbbcfe6f7e63e766 TW-30589 fix create temp dir race condition diff -r f6efcbc52d11 -r 7ea82d063131 mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtil.java --- 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 myTmpDirLocks = new ConcurrentHashMap(); 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; } diff -r f6efcbc52d11 -r 7ea82d063131 mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/HgFileUtilTest.java --- 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 tmpFiles = new HashSet(); + Runnable createTmpFile = new Runnable() { + public void run() { + try { + allThreadsInitialized.await(); + tmpFiles.add(HgFileUtil.createTempDir()); + } catch (Exception e) { + //ignore + } + } + }; + + List threads = new ArrayList(); + 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()); + } + }