[libcxx-commits] [libcxx] b982c6f - [libcxx] [test] Avoid race conditions between tests regarding temp directories

[Martin Storsjö via libcxx-commits]

Author: Martin Storsjö
Date: 2021-03-19T16:12:24+02:00
New Revision: b982c6f5fa1bd8762554dbc79bf16b9449ca095a

URL: https://github.com/llvm/llvm-project/commit/b982c6f5fa1bd8762554dbc79bf16b9449ca095a
DIFF: https://github.com/llvm/llvm-project/commit/b982c6f5fa1bd8762554dbc79bf16b9449ca095a.diff

LOG: [libcxx] [test] Avoid race conditions between tests regarding temp directories

Prior to e0d01294bc124211a8ffb55e69162eb34a242680, all tests used a
random directory name, but now it is deterministic, based on the
test name. This change was done under the assumption that the filename
portion of the cwd is unique across tests that use the filesystem
test temporary directories.

When running tests locally, the cwd of the test is something like
"<build-dir>/test/<test path>/Output/copy_assign.pass.cpp.dir",
and the filename portion, "copy_assign.pass.cpp.dir", is used as
base for the temp directory names.

The change noted that there's a risk for race conditions if multiple
threads within one test try to create temp directories in parallel, but
that doesn't really happen in practice.

However, if running tests with a large number of parallel workers,
multiple tests with the same filename portion, e.g. "copy_assign.pass.cpp.dir",
can run in parallel, leading to race conditions across processes.

Therefore, add a hash of the full cwd to distinguish such cases
from each other.

Secondly, don't use two separate levels of temporary directories
(<base>/static_env.0). When cleaning up, only the individual
directory is removed, leaving the empty intermediate directory
behind littering the temp directory.

Differential Revision: https://reviews.llvm.org/D98703

Added: 
    

Modified: 
    libcxx/test/support/filesystem_test_helper.h

Removed: 
    


################################################################################
diff  --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index e1607fd61899..e87c43b9ea09 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -296,14 +296,17 @@ struct scoped_test_env
     // sharing the same cwd). However, it is fairly unlikely to happen as
     // we generally don't use scoped_test_env from multiple threads, so
     // this is deemed acceptable.
+    // The cwd.filename() itself isn't unique across all tests in the suite,
+    // so start the numbering from a hash of the full cwd, to avoid
+    // 
diff erent tests interfering with each other.
     static inline fs::path available_cwd_path() {
         fs::path const cwd = utils::getcwd();
         fs::path const tmp = fs::temp_directory_path();
-        fs::path const base = tmp / cwd.filename();
-        int i = 0;
-        fs::path p = base / ("static_env." + std::to_string(i));
+        std::string base = cwd.filename().string();
+        size_t i = std::hash<std::string>()(cwd.string());
+        fs::path p = tmp / (base + "-static_env." + std::to_string(i));
         while (utils::exists(p.string())) {
-            p = fs::path(base) / ("static_env." + std::to_string(++i));
+            p = tmp / (base + "-static_env." + std::to_string(++i));
         }
         return p;
     }