[clang] [llvm] [Support][Modules] Ensure parent directory exists when creating lock file (PR #189888)
Aadarsh Keshri via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 4 01:46:10 PDT 2026
https://github.com/Aadarsh-Keshri updated https://github.com/llvm/llvm-project/pull/189888
>From 440e267a7027514bc00850a8cbbe1ad45b696645 Mon Sep 17 00:00:00 2001
From: Aadarsh-Keshri <keshri.aadarsh123 at gmail.com>
Date: Wed, 1 Apr 2026 12:11:32 +0530
Subject: [PATCH 1/2] Ensure parent directory exists when creating lock file
---
clang/lib/Serialization/ModuleCache.cpp | 5 ----
llvm/lib/Support/LockFileManager.cpp | 27 ++++++++++++++++--
.../unittests/Support/LockFileManagerTest.cpp | 28 +++++++++++++++++++
3 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 658da6e3b7145..7c57d143d2e03 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -109,11 +109,6 @@ class CrossProcessModuleCache : public ModuleCache {
void prepareForGetLock(StringRef ModuleFilename) override {
// This is a compiler-internal input/output, let's bypass the sandbox.
auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
-
- // FIXME: Do this in LockFileManager and only if the directory doesn't
- // exist.
- StringRef Dir = llvm::sys::path::parent_path(ModuleFilename);
- llvm::sys::fs::create_directories(Dir);
}
std::unique_ptr<llvm::AdvisoryLock>
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 1a98ec03486db..205339f9366bd 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -16,6 +16,7 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/IOSandbox.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/Process.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
@@ -185,10 +186,30 @@ Expected<bool> LockFileManager::tryLock() {
UniqueLockFileName = LockFileName;
UniqueLockFileName += "-%%%%%%%%";
int UniqueLockFileID;
- if (std::error_code EC = sys::fs::createUniqueFile(
- UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
- return createStringError(EC, "failed to create unique file " +
+
+ {
+ std::error_code EC = sys::fs::createUniqueFile(
+ UniqueLockFileName, UniqueLockFileID, UniqueLockFileName);
+ if (EC == errc::no_such_file_or_directory) {
+ SmallString<128> Dir = sys::path::parent_path(UniqueLockFileName);
+ if (!Dir.empty()) {
+ if (std::error_code DirEC = sys::fs::create_directories(Dir))
+ return createStringError(DirEC,
+ "failed to create lock directory " + Dir);
+ }
+
+ // Retry creating lock file
+ UniqueLockFileName = LockFileName;
+ UniqueLockFileName += "-%%%%%%%%";
+
+ EC = sys::fs::createUniqueFile(UniqueLockFileName, UniqueLockFileID,
UniqueLockFileName);
+ }
+
+ if (EC)
+ return createStringError(EC, "failed to create unique file " +
+ UniqueLockFileName);
+ }
// Clean up the unique file on signal or scope exit.
RemoveUniqueLockFileOnSignal RemoveUniqueFile(UniqueLockFileName);
diff --git a/llvm/unittests/Support/LockFileManagerTest.cpp b/llvm/unittests/Support/LockFileManagerTest.cpp
index bd61b6c36efb3..ce5fd4d5063ce 100644
--- a/llvm/unittests/Support/LockFileManagerTest.cpp
+++ b/llvm/unittests/Support/LockFileManagerTest.cpp
@@ -104,4 +104,32 @@ TEST(LockFileManagerTest, RelativePath) {
ASSERT_FALSE(chdir(OrigPath));
}
+TEST(LockFileManagerTest, NonExistentDirectory) {
+ TempDir LockFileManagerTestDir("LockFileManagerTestDir", /*Unique*/ true);
+
+ SmallString<64> NonExistentDir(LockFileManagerTestDir.path());
+ sys::path::append(NonExistentDir, "does_not_exist");
+
+ SmallString<64> LockedFile(NonExistentDir);
+ sys::path::append(LockedFile, "file");
+
+ SmallString<64> FileLock(LockedFile);
+ FileLock += ".lock";
+
+ // Ensure directory truly does not exist before test
+ EXPECT_FALSE(sys::fs::exists(NonExistentDir));
+
+ {
+ LockFileManager Locked(LockedFile);
+ EXPECT_THAT_EXPECTED(Locked.tryLock(), HasValue(true));
+
+ // Directory and lock file should now exist
+ EXPECT_TRUE(sys::fs::exists(NonExistentDir));
+ EXPECT_TRUE(sys::fs::exists(FileLock));
+ }
+
+ // After destruction, lock file should be removed
+ EXPECT_FALSE(sys::fs::exists(FileLock));
+}
+
} // end anonymous namespace
>From 4ce3c51a66bc2484f99e6266b5fce987c83974aa Mon Sep 17 00:00:00 2001
From: Aadarsh-Keshri <keshri.aadarsh123 at gmail.com>
Date: Sat, 4 Apr 2026 14:05:16 +0530
Subject: [PATCH 2/2] Deleted prepareForGetLock and its usages. Tweaked lock
file creation logic
---
.../include/clang/Serialization/ModuleCache.h | 4 ---
.../DependencyScannerImpl.cpp | 1 -
.../InProcessModuleCache.cpp | 2 --
clang/lib/Frontend/CompilerInstance.cpp | 1 -
clang/lib/Serialization/ModuleCache.cpp | 6 +----
llvm/lib/Support/LockFileManager.cpp | 25 ++++++++++---------
6 files changed, 14 insertions(+), 25 deletions(-)
diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h
index c6795c5dc358a..621bde0829f74 100644
--- a/clang/include/clang/Serialization/ModuleCache.h
+++ b/clang/include/clang/Serialization/ModuleCache.h
@@ -24,10 +24,6 @@ class InMemoryModuleCache;
/// operations the compiler might want to perform on the cache.
class ModuleCache {
public:
- /// May perform any work that only needs to be performed once for multiple
- /// calls \c getLock() with the same module filename.
- virtual void prepareForGetLock(StringRef ModuleFilename) = 0;
-
/// Returns lock for the given module file. The lock is initially unlocked.
virtual std::unique_ptr<llvm::AdvisoryLock>
getLock(StringRef ModuleFilename) = 0;
diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
index e2fea6f86017a..1940d2f275c73 100644
--- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
@@ -657,7 +657,6 @@ struct AsyncModuleCompile : PPCallbacks {
return;
}
- ModCache.prepareForGetLock(ModuleFileName);
auto Lock = ModCache.getLock(ModuleFileName);
bool Owned;
llvm::Error LockErr = Lock->tryLock().moveInto(Owned);
diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
index cd7385c8f38c2..05e8591129981 100644
--- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp
+++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp
@@ -82,8 +82,6 @@ class InProcessModuleCache : public ModuleCache {
public:
InProcessModuleCache(ModuleCacheEntries &Entries) : Entries(Entries) {}
- void prepareForGetLock(StringRef Filename) override {}
-
std::unique_ptr<llvm::AdvisoryLock> getLock(StringRef Filename) override {
auto &Entry = [&]() -> ModuleCacheEntry & {
std::lock_guard<std::mutex> Lock(Entries.Mutex);
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 33919bc1c4634..bad85491e1362 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1491,7 +1491,6 @@ compileModuleBehindLockOrRead(CompilerInstance &ImportingInstance,
<< ModuleFileName << Module->Name;
auto &ModuleCache = ImportingInstance.getModuleCache();
- ModuleCache.prepareForGetLock(ModuleFileName);
while (true) {
auto Lock = ModuleCache.getLock(ModuleFileName);
diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp
index 7c57d143d2e03..16c533e587177 100644
--- a/clang/lib/Serialization/ModuleCache.cpp
+++ b/clang/lib/Serialization/ModuleCache.cpp
@@ -106,13 +106,9 @@ class CrossProcessModuleCache : public ModuleCache {
InMemoryModuleCache InMemory;
public:
- void prepareForGetLock(StringRef ModuleFilename) override {
- // This is a compiler-internal input/output, let's bypass the sandbox.
- auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
- }
-
std::unique_ptr<llvm::AdvisoryLock>
getLock(StringRef ModuleFilename) override {
+ auto BypassSandbox = llvm::sys::sandbox::scopedDisable();
return std::make_unique<llvm::LockFileManager>(ModuleFilename);
}
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 205339f9366bd..e948b6ced5ae4 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -183,15 +183,17 @@ Expected<bool> LockFileManager::tryLock() {
}
// Create a lock file that is unique to this instance.
- UniqueLockFileName = LockFileName;
- UniqueLockFileName += "-%%%%%%%%";
+ SmallString<128> UniqueLockFilePattern = LockFileName;
+ UniqueLockFilePattern += "-%%%%%%%%";
+
int UniqueLockFileID;
{
- std::error_code EC = sys::fs::createUniqueFile(
- UniqueLockFileName, UniqueLockFileID, UniqueLockFileName);
+ SmallString<128> UniquePath = UniqueLockFilePattern;
+ std::error_code EC =
+ sys::fs::createUniqueFile(UniquePath, UniqueLockFileID, UniquePath);
if (EC == errc::no_such_file_or_directory) {
- SmallString<128> Dir = sys::path::parent_path(UniqueLockFileName);
+ SmallString<128> Dir = sys::path::parent_path(UniqueLockFilePattern);
if (!Dir.empty()) {
if (std::error_code DirEC = sys::fs::create_directories(Dir))
return createStringError(DirEC,
@@ -199,16 +201,15 @@ Expected<bool> LockFileManager::tryLock() {
}
// Retry creating lock file
- UniqueLockFileName = LockFileName;
- UniqueLockFileName += "-%%%%%%%%";
-
- EC = sys::fs::createUniqueFile(UniqueLockFileName, UniqueLockFileID,
- UniqueLockFileName);
+ UniquePath = UniqueLockFilePattern;
+ EC = sys::fs::createUniqueFile(UniquePath, UniqueLockFileID, UniquePath);
}
if (EC)
- return createStringError(EC, "failed to create unique file " +
- UniqueLockFileName);
+ return createStringError(EC,
+ "failed to create unique file " + UniquePath);
+
+ UniqueLockFileName = UniquePath;
}
// Clean up the unique file on signal or scope exit.
More information about the cfe-commits
mailing list