https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/130834
>From 834eac8d3ce286172456e2a177d0e5284ade5b46 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Tue, 11 Mar 2025 09:17:50 -0700
Subject: [PATCH 1/2] [Support] Do not remove lock file on failure
---
clang/lib/Frontend/CompilerInstance.cpp | 2 --
llvm/lib/Support/LockFileManager.cpp | 14 ++++++++------
llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp | 1 -
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInstance.cpp
b/clang/lib/Frontend/CompilerInstance.cpp
index e9e96683ca51f..44f4f48ef94e8 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1491,8 +1491,6 @@ static bool compileModuleAndReadASTBehindLock(
// related errors.
Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
<< Module->Name << toString(std::move(Err));
- // Clear out any potential leftover.
- Lock.unsafeRemoveLockFile();
return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
ModuleNameLoc, Module,
ModuleFileName);
}
diff --git a/llvm/lib/Support/LockFileManager.cpp
b/llvm/lib/Support/LockFileManager.cpp
index 7cf9db379974f..cbffd11e547bd 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -169,7 +169,7 @@ Expected<bool> LockFileManager::tryLock() {
SmallString<128> AbsoluteFileName(FileName);
if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
return createStringError(EC, "failed to obtain absolute path for " +
- AbsoluteFileName);
+ AbsoluteFileName); // We don't even know
the LockFileName yet.
LockFileName = AbsoluteFileName;
LockFileName += ".lock";
@@ -180,6 +180,8 @@ Expected<bool> LockFileManager::tryLock() {
return false;
}
+ // LockFileName either does not exist or we could not read it and removed it
in readLockFile().
+
// Create a lock file that is unique to this instance.
UniqueLockFileName = LockFileName;
UniqueLockFileName += "-%%%%%%%%";
@@ -187,13 +189,13 @@ Expected<bool> LockFileManager::tryLock() {
if (std::error_code EC = sys::fs::createUniqueFile(
UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
return createStringError(EC, "failed to create unique file " +
- UniqueLockFileName);
+ UniqueLockFileName); // LockFileName
still does not exist.
// Write our process ID to our unique lock file.
{
SmallString<256> HostID;
if (auto EC = getHostID(HostID))
- return createStringError(EC, "failed to get host id");
+ return createStringError(EC, "failed to get host id"); // LockFileName
still does not exist. We may leave behind UniqueLockFileName though, this that
in a follow-up.
raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
Out << HostID << ' ' << sys::Process::getProcessId();
@@ -207,7 +209,7 @@ Expected<bool> LockFileManager::tryLock() {
sys::fs::remove(UniqueLockFileName);
// Don't call report_fatal_error.
Out.clear_error();
- return std::move(Err);
+ return std::move(Err); // LockFileName still does not exist.
}
}
@@ -227,7 +229,7 @@ Expected<bool> LockFileManager::tryLock() {
if (EC != errc::file_exists)
return createStringError(EC, "failed to create link " + LockFileName +
- " to " + UniqueLockFileName);
+ " to " + UniqueLockFileName); // We
failed to create LockFileName for a weird reason.
// Someone else managed to create the lock file first. Read the process ID
// from the lock file.
@@ -248,7 +250,7 @@ Expected<bool> LockFileManager::tryLock() {
// ownership.
if ((EC = sys::fs::remove(LockFileName)))
return createStringError(EC, "failed to remove lockfile " +
- UniqueLockFileName);
+ UniqueLockFileName); // Failed to
remove LockFileName. Why try to do it again?
}
}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
index 65cbb13628301..e2e449f1c8a38 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -1552,7 +1552,6 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
LLVM_DEBUG(
dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug
"
"output may be mangled by other processes\n");
- Lock.unsafeRemoveLockFile();
} else if (!Owned) {
switch (Lock.waitForUnlock()) {
case LockFileManager::Res_Success:
>From 1c1c3cbae946e4c652c2c71ff600c42d1f26b896 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Tue, 11 Mar 2025 15:46:30 -0700
Subject: [PATCH 2/2] Remove notes
---
llvm/lib/Support/LockFileManager.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Support/LockFileManager.cpp
b/llvm/lib/Support/LockFileManager.cpp
index cbffd11e547bd..7cf9db379974f 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -169,7 +169,7 @@ Expected<bool> LockFileManager::tryLock() {
SmallString<128> AbsoluteFileName(FileName);
if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
return createStringError(EC, "failed to obtain absolute path for " +
- AbsoluteFileName); // We don't even know
the LockFileName yet.
+ AbsoluteFileName);
LockFileName = AbsoluteFileName;
LockFileName += ".lock";
@@ -180,8 +180,6 @@ Expected<bool> LockFileManager::tryLock() {
return false;
}
- // LockFileName either does not exist or we could not read it and removed it
in readLockFile().
-
// Create a lock file that is unique to this instance.
UniqueLockFileName = LockFileName;
UniqueLockFileName += "-%%%%%%%%";
@@ -189,13 +187,13 @@ Expected<bool> LockFileManager::tryLock() {
if (std::error_code EC = sys::fs::createUniqueFile(
UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
return createStringError(EC, "failed to create unique file " +
- UniqueLockFileName); // LockFileName
still does not exist.
+ UniqueLockFileName);
// Write our process ID to our unique lock file.
{
SmallString<256> HostID;
if (auto EC = getHostID(HostID))
- return createStringError(EC, "failed to get host id"); // LockFileName
still does not exist. We may leave behind UniqueLockFileName though, this that
in a follow-up.
+ return createStringError(EC, "failed to get host id");
raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
Out << HostID << ' ' << sys::Process::getProcessId();
@@ -209,7 +207,7 @@ Expected<bool> LockFileManager::tryLock() {
sys::fs::remove(UniqueLockFileName);
// Don't call report_fatal_error.
Out.clear_error();
- return std::move(Err); // LockFileName still does not exist.
+ return std::move(Err);
}
}
@@ -229,7 +227,7 @@ Expected<bool> LockFileManager::tryLock() {
if (EC != errc::file_exists)
return createStringError(EC, "failed to create link " + LockFileName +
- " to " + UniqueLockFileName); // We
failed to create LockFileName for a weird reason.
+ " to " + UniqueLockFileName);
// Someone else managed to create the lock file first. Read the process ID
// from the lock file.
@@ -250,7 +248,7 @@ Expected<bool> LockFileManager::tryLock() {
// ownership.
if ((EC = sys::fs::remove(LockFileName)))
return createStringError(EC, "failed to remove lockfile " +
- UniqueLockFileName); // Failed to
remove LockFileName. Why try to do it again?
+ UniqueLockFileName);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
