[Support] Return LockFileManager errors right away
#130627
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-clang-modules Author: Jan Svoboda (jansvoboda11) ChangesThis patch removes some internal state out of Full diff: https://github.com/llvm/llvm-project/pull/130627.diff 5 Files Affected:
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c11c857ea0606..fe351e6b41342 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1477,29 +1477,28 @@ static bool compileModuleAndReadASTBehindLock(
llvm::sys::fs::create_directories(Dir);
while (true) {
- llvm::LockFileManager Locked(ModuleFileName);
- switch (Locked) {
- case llvm::LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(ModuleFileName);
+ bool Owned;
+ if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
// ModuleCache takes care of correctness and locks are only necessary for
// performance. Fallback to building the module in case of any lock
// related errors.
Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
- << Module->Name << Locked.getErrorMessage();
+ << Module->Name << toString(std::move(Err));
// Clear out any potential leftover.
- Locked.unsafeRemoveLockFile();
- [[fallthrough]];
- case llvm::LockFileManager::LFS_Owned:
+ Lock.unsafeRemoveLockFile();
+ return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
+ ModuleNameLoc, Module, ModuleFileName);
+ }
+ if (Owned) {
// We're responsible for building the module ourselves.
return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
ModuleNameLoc, Module, ModuleFileName);
-
- case llvm::LockFileManager::LFS_Shared:
- break; // The interesting case.
}
// Someone else is responsible for building the module. Wait for them to
// finish.
- switch (Locked.waitForUnlock()) {
+ switch (Lock.waitForUnlock()) {
case llvm::LockFileManager::Res_Success:
break; // The interesting case.
case llvm::LockFileManager::Res_OwnerDied:
@@ -1511,7 +1510,7 @@ static bool compileModuleAndReadASTBehindLock(
Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
<< Module->Name;
// Clear the lock file so that future invocations can make progress.
- Locked.unsafeRemoveLockFile();
+ Lock.unsafeRemoveLockFile();
continue;
}
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp
index 4b920fccecac3..1e2272c48bd04 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -849,22 +849,21 @@ GlobalModuleIndex::writeIndex(FileManager &FileMgr,
// Coordinate building the global index file with other processes that might
// try to do the same.
- llvm::LockFileManager Locked(IndexPath);
- switch (Locked) {
- case llvm::LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(IndexPath);
+ bool Owned;
+ if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
+ llvm::consumeError(std::move(Err));
return llvm::createStringError(std::errc::io_error, "LFS error");
-
- case llvm::LockFileManager::LFS_Owned:
- // We're responsible for building the index ourselves. Do so below.
- break;
-
- case llvm::LockFileManager::LFS_Shared:
+ }
+ if (!Owned) {
// Someone else is responsible for building the index. We don't care
// when they finish, so we're done.
return llvm::createStringError(std::errc::device_or_resource_busy,
"someone else is building the index");
}
+ // We're responsible for building the index ourselves.
+
// The module index builder.
GlobalModuleIndexBuilder Builder(FileMgr, PCHContainerRdr);
diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index 92c7ceed6a929..e687f3b1dfa53 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -9,6 +9,7 @@
#define LLVM_SUPPORT_LOCKFILEMANAGER_H
#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Error.h"
#include <optional>
#include <system_error>
#include <utility> // for std::pair
@@ -26,19 +27,6 @@ class StringRef;
/// operation.
class LockFileManager {
public:
- /// Describes the state of a lock file.
- enum LockFileState {
- /// The lock file has been created and is owned by this instance
- /// of the object.
- LFS_Owned,
- /// The lock file already exists and is owned by some other
- /// instance.
- LFS_Shared,
- /// An error occurred while trying to create or find the lock
- /// file.
- LFS_Error
- };
-
/// Describes the result of waiting for the owner to release the lock.
enum WaitForUnlockResult {
/// The lock was released successfully.
@@ -55,8 +43,6 @@ class LockFileManager {
SmallString<128> UniqueLockFileName;
std::optional<std::pair<std::string, int>> Owner;
- std::error_code ErrorCode;
- std::string ErrorDiagMsg;
LockFileManager(const LockFileManager &) = delete;
LockFileManager &operator=(const LockFileManager &) = delete;
@@ -71,10 +57,10 @@ class LockFileManager {
LockFileManager(StringRef FileName);
~LockFileManager();
- /// Determine the state of the lock file.
- LockFileState getState() const;
-
- operator LockFileState() const { return getState(); }
+ /// Tries to acquire the lock without blocking.
+ /// \returns true if the lock was successfully acquired, false if the lock is
+ /// already held by someone else, or \c Error in case of unexpected failure.
+ Expected<bool> tryLock();
/// For a shared lock, wait until the owner releases the lock.
/// Total timeout for the file to appear is ~1.5 minutes.
@@ -84,15 +70,6 @@ class LockFileManager {
/// Remove the lock file. This may delete a different lock file than
/// the one previously read if there is a race.
std::error_code unsafeRemoveLockFile();
-
- /// Get error message, or "" if there is no error.
- std::string getErrorMessage() const;
-
- /// Set error and error message
- void setError(const std::error_code &EC, StringRef ErrorMsg = "") {
- ErrorCode = EC;
- ErrorDiagMsg = ErrorMsg.str();
- }
};
} // end namespace llvm
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 9a45a9966458e..1ec206415a13c 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -157,42 +157,35 @@ class RemoveUniqueLockFileOnSignal {
} // end anonymous namespace
-LockFileManager::LockFileManager(StringRef FileName)
-{
- this->FileName = FileName;
- if (std::error_code EC = sys::fs::make_absolute(this->FileName)) {
- std::string S("failed to obtain absolute path for ");
- S.append(std::string(this->FileName));
- setError(EC, S);
- return;
- }
- LockFileName = this->FileName;
+LockFileManager::LockFileManager(StringRef FileName) : FileName(FileName) {}
+
+Expected<bool> LockFileManager::tryLock() {
+ SmallString<128> AbsoluteFileName(FileName);
+ if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
+ return createStringError("failed to obtain absolute path for " +
+ AbsoluteFileName);
+ LockFileName = AbsoluteFileName;
LockFileName += ".lock";
// If the lock file already exists, don't bother to try to create our own
// lock file; it won't work anyway. Just figure out who owns this lock file.
if ((Owner = readLockFile(LockFileName)))
- return;
+ return false;
// Create a lock file that is unique to this instance.
UniqueLockFileName = LockFileName;
UniqueLockFileName += "-%%%%%%%%";
int UniqueLockFileID;
if (std::error_code EC = sys::fs::createUniqueFile(
- UniqueLockFileName, UniqueLockFileID, UniqueLockFileName)) {
- std::string S("failed to create unique file ");
- S.append(std::string(UniqueLockFileName));
- setError(EC, S);
- return;
- }
+ UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
+ return createStringError("failed to create unique file " +
+ UniqueLockFileName);
// Write our process ID to our unique lock file.
{
SmallString<256> HostID;
- if (auto EC = getHostID(HostID)) {
- setError(EC, "failed to get host id");
- return;
- }
+ if (auto EC = getHostID(HostID))
+ return createStringError(EC, "failed to get host id");
raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
Out << HostID << ' ' << sys::Process::getProcessId();
@@ -201,13 +194,12 @@ LockFileManager::LockFileManager(StringRef FileName)
if (Out.has_error()) {
// We failed to write out PID, so report the error, remove the
// unique lock file, and fail.
- std::string S("failed to write to ");
- S.append(std::string(UniqueLockFileName));
- setError(Out.error(), S);
+ Error Err = createStringError(Out.error(),
+ "failed to write to " + UniqueLockFileName);
sys::fs::remove(UniqueLockFileName);
// Don't call report_fatal_error.
Out.clear_error();
- return;
+ return std::move(Err);
}
}
@@ -221,23 +213,19 @@ LockFileManager::LockFileManager(StringRef FileName)
sys::fs::create_link(UniqueLockFileName, LockFileName);
if (!EC) {
RemoveUniqueFile.lockAcquired();
- return;
+ return true;
}
- if (EC != errc::file_exists) {
- std::string S("failed to create link ");
- raw_string_ostream OSS(S);
- OSS << LockFileName.str() << " to " << UniqueLockFileName.str();
- setError(EC, S);
- return;
- }
+ if (EC != errc::file_exists)
+ return createStringError(EC, "failed to create link " + LockFileName +
+ " to " + UniqueLockFileName);
// Someone else managed to create the lock file first. Read the process ID
// from the lock file.
if ((Owner = readLockFile(LockFileName))) {
// Wipe out our unique lock file (it's useless now)
sys::fs::remove(UniqueLockFileName);
- return;
+ return false;
}
if (!sys::fs::exists(LockFileName)) {
@@ -248,39 +236,14 @@ LockFileManager::LockFileManager(StringRef FileName)
// There is a lock file that nobody owns; try to clean it up and get
// ownership.
- if ((EC = sys::fs::remove(LockFileName))) {
- std::string S("failed to remove lockfile ");
- S.append(std::string(UniqueLockFileName));
- setError(EC, S);
- return;
- }
- }
-}
-
-LockFileManager::LockFileState LockFileManager::getState() const {
- if (Owner)
- return LFS_Shared;
-
- if (ErrorCode)
- return LFS_Error;
-
- return LFS_Owned;
-}
-
-std::string LockFileManager::getErrorMessage() const {
- if (ErrorCode) {
- std::string Str(ErrorDiagMsg);
- std::string ErrCodeMsg = ErrorCode.message();
- raw_string_ostream OSS(Str);
- if (!ErrCodeMsg.empty())
- OSS << ": " << ErrCodeMsg;
- return Str;
+ if ((EC = sys::fs::remove(LockFileName)))
+ return createStringError(EC, "failed to remove lockfile " +
+ UniqueLockFileName);
}
- return "";
}
LockFileManager::~LockFileManager() {
- if (getState() != LFS_Owned)
+ if (Owner)
return;
// Since we own the lock, remove the lock file and our own unique lock file.
@@ -293,7 +256,7 @@ LockFileManager::~LockFileManager() {
LockFileManager::WaitForUnlockResult
LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
- if (getState() != LFS_Shared)
+ if (!Owner)
return Res_Success;
// Since we don't yet have an event-based method to wait for the lock file,
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
index 65aee2f70104b..65cbb13628301 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -1545,18 +1545,16 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
<< "'\n");
while (true) {
- llvm::LockFileManager Locked(LockFilePath.str());
- switch (Locked) {
- case LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(LockFilePath.str());
+ bool Owned;
+ if (Error Err = Lock.tryLock().moveInto(Owned)) {
+ consumeError(std::move(Err));
LLVM_DEBUG(
dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug "
"output may be mangled by other processes\n");
- Locked.unsafeRemoveLockFile();
- break;
- case LockFileManager::LFS_Owned:
- break;
- case LockFileManager::LFS_Shared: {
- switch (Locked.waitForUnlock()) {
+ Lock.unsafeRemoveLockFile();
+ } else if (!Owned) {
+ switch (Lock.waitForUnlock()) {
case LockFileManager::Res_Success:
break;
case LockFileManager::Res_OwnerDied:
@@ -1566,11 +1564,9 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
dbgs()
<< "[amdgpu-split-module] unable to acquire lockfile, debug "
"output may be mangled by other processes\n");
- Locked.unsafeRemoveLockFile();
+ Lock.unsafeRemoveLockFile();
break; // give up
}
- break;
- }
}
splitAMDGPUModule(TTIGetter, M, N, ModuleCallback);
|
|
@llvm/pr-subscribers-backend-amdgpu Author: Jan Svoboda (jansvoboda11) ChangesThis patch removes some internal state out of Full diff: https://github.com/llvm/llvm-project/pull/130627.diff 5 Files Affected:
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index c11c857ea0606..fe351e6b41342 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1477,29 +1477,28 @@ static bool compileModuleAndReadASTBehindLock(
llvm::sys::fs::create_directories(Dir);
while (true) {
- llvm::LockFileManager Locked(ModuleFileName);
- switch (Locked) {
- case llvm::LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(ModuleFileName);
+ bool Owned;
+ if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
// ModuleCache takes care of correctness and locks are only necessary for
// performance. Fallback to building the module in case of any lock
// related errors.
Diags.Report(ModuleNameLoc, diag::remark_module_lock_failure)
- << Module->Name << Locked.getErrorMessage();
+ << Module->Name << toString(std::move(Err));
// Clear out any potential leftover.
- Locked.unsafeRemoveLockFile();
- [[fallthrough]];
- case llvm::LockFileManager::LFS_Owned:
+ Lock.unsafeRemoveLockFile();
+ return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
+ ModuleNameLoc, Module, ModuleFileName);
+ }
+ if (Owned) {
// We're responsible for building the module ourselves.
return compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
ModuleNameLoc, Module, ModuleFileName);
-
- case llvm::LockFileManager::LFS_Shared:
- break; // The interesting case.
}
// Someone else is responsible for building the module. Wait for them to
// finish.
- switch (Locked.waitForUnlock()) {
+ switch (Lock.waitForUnlock()) {
case llvm::LockFileManager::Res_Success:
break; // The interesting case.
case llvm::LockFileManager::Res_OwnerDied:
@@ -1511,7 +1510,7 @@ static bool compileModuleAndReadASTBehindLock(
Diags.Report(ModuleNameLoc, diag::remark_module_lock_timeout)
<< Module->Name;
// Clear the lock file so that future invocations can make progress.
- Locked.unsafeRemoveLockFile();
+ Lock.unsafeRemoveLockFile();
continue;
}
diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp
index 4b920fccecac3..1e2272c48bd04 100644
--- a/clang/lib/Serialization/GlobalModuleIndex.cpp
+++ b/clang/lib/Serialization/GlobalModuleIndex.cpp
@@ -849,22 +849,21 @@ GlobalModuleIndex::writeIndex(FileManager &FileMgr,
// Coordinate building the global index file with other processes that might
// try to do the same.
- llvm::LockFileManager Locked(IndexPath);
- switch (Locked) {
- case llvm::LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(IndexPath);
+ bool Owned;
+ if (llvm::Error Err = Lock.tryLock().moveInto(Owned)) {
+ llvm::consumeError(std::move(Err));
return llvm::createStringError(std::errc::io_error, "LFS error");
-
- case llvm::LockFileManager::LFS_Owned:
- // We're responsible for building the index ourselves. Do so below.
- break;
-
- case llvm::LockFileManager::LFS_Shared:
+ }
+ if (!Owned) {
// Someone else is responsible for building the index. We don't care
// when they finish, so we're done.
return llvm::createStringError(std::errc::device_or_resource_busy,
"someone else is building the index");
}
+ // We're responsible for building the index ourselves.
+
// The module index builder.
GlobalModuleIndexBuilder Builder(FileMgr, PCHContainerRdr);
diff --git a/llvm/include/llvm/Support/LockFileManager.h b/llvm/include/llvm/Support/LockFileManager.h
index 92c7ceed6a929..e687f3b1dfa53 100644
--- a/llvm/include/llvm/Support/LockFileManager.h
+++ b/llvm/include/llvm/Support/LockFileManager.h
@@ -9,6 +9,7 @@
#define LLVM_SUPPORT_LOCKFILEMANAGER_H
#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Error.h"
#include <optional>
#include <system_error>
#include <utility> // for std::pair
@@ -26,19 +27,6 @@ class StringRef;
/// operation.
class LockFileManager {
public:
- /// Describes the state of a lock file.
- enum LockFileState {
- /// The lock file has been created and is owned by this instance
- /// of the object.
- LFS_Owned,
- /// The lock file already exists and is owned by some other
- /// instance.
- LFS_Shared,
- /// An error occurred while trying to create or find the lock
- /// file.
- LFS_Error
- };
-
/// Describes the result of waiting for the owner to release the lock.
enum WaitForUnlockResult {
/// The lock was released successfully.
@@ -55,8 +43,6 @@ class LockFileManager {
SmallString<128> UniqueLockFileName;
std::optional<std::pair<std::string, int>> Owner;
- std::error_code ErrorCode;
- std::string ErrorDiagMsg;
LockFileManager(const LockFileManager &) = delete;
LockFileManager &operator=(const LockFileManager &) = delete;
@@ -71,10 +57,10 @@ class LockFileManager {
LockFileManager(StringRef FileName);
~LockFileManager();
- /// Determine the state of the lock file.
- LockFileState getState() const;
-
- operator LockFileState() const { return getState(); }
+ /// Tries to acquire the lock without blocking.
+ /// \returns true if the lock was successfully acquired, false if the lock is
+ /// already held by someone else, or \c Error in case of unexpected failure.
+ Expected<bool> tryLock();
/// For a shared lock, wait until the owner releases the lock.
/// Total timeout for the file to appear is ~1.5 minutes.
@@ -84,15 +70,6 @@ class LockFileManager {
/// Remove the lock file. This may delete a different lock file than
/// the one previously read if there is a race.
std::error_code unsafeRemoveLockFile();
-
- /// Get error message, or "" if there is no error.
- std::string getErrorMessage() const;
-
- /// Set error and error message
- void setError(const std::error_code &EC, StringRef ErrorMsg = "") {
- ErrorCode = EC;
- ErrorDiagMsg = ErrorMsg.str();
- }
};
} // end namespace llvm
diff --git a/llvm/lib/Support/LockFileManager.cpp b/llvm/lib/Support/LockFileManager.cpp
index 9a45a9966458e..1ec206415a13c 100644
--- a/llvm/lib/Support/LockFileManager.cpp
+++ b/llvm/lib/Support/LockFileManager.cpp
@@ -157,42 +157,35 @@ class RemoveUniqueLockFileOnSignal {
} // end anonymous namespace
-LockFileManager::LockFileManager(StringRef FileName)
-{
- this->FileName = FileName;
- if (std::error_code EC = sys::fs::make_absolute(this->FileName)) {
- std::string S("failed to obtain absolute path for ");
- S.append(std::string(this->FileName));
- setError(EC, S);
- return;
- }
- LockFileName = this->FileName;
+LockFileManager::LockFileManager(StringRef FileName) : FileName(FileName) {}
+
+Expected<bool> LockFileManager::tryLock() {
+ SmallString<128> AbsoluteFileName(FileName);
+ if (std::error_code EC = sys::fs::make_absolute(AbsoluteFileName))
+ return createStringError("failed to obtain absolute path for " +
+ AbsoluteFileName);
+ LockFileName = AbsoluteFileName;
LockFileName += ".lock";
// If the lock file already exists, don't bother to try to create our own
// lock file; it won't work anyway. Just figure out who owns this lock file.
if ((Owner = readLockFile(LockFileName)))
- return;
+ return false;
// Create a lock file that is unique to this instance.
UniqueLockFileName = LockFileName;
UniqueLockFileName += "-%%%%%%%%";
int UniqueLockFileID;
if (std::error_code EC = sys::fs::createUniqueFile(
- UniqueLockFileName, UniqueLockFileID, UniqueLockFileName)) {
- std::string S("failed to create unique file ");
- S.append(std::string(UniqueLockFileName));
- setError(EC, S);
- return;
- }
+ UniqueLockFileName, UniqueLockFileID, UniqueLockFileName))
+ return createStringError("failed to create unique file " +
+ UniqueLockFileName);
// Write our process ID to our unique lock file.
{
SmallString<256> HostID;
- if (auto EC = getHostID(HostID)) {
- setError(EC, "failed to get host id");
- return;
- }
+ if (auto EC = getHostID(HostID))
+ return createStringError(EC, "failed to get host id");
raw_fd_ostream Out(UniqueLockFileID, /*shouldClose=*/true);
Out << HostID << ' ' << sys::Process::getProcessId();
@@ -201,13 +194,12 @@ LockFileManager::LockFileManager(StringRef FileName)
if (Out.has_error()) {
// We failed to write out PID, so report the error, remove the
// unique lock file, and fail.
- std::string S("failed to write to ");
- S.append(std::string(UniqueLockFileName));
- setError(Out.error(), S);
+ Error Err = createStringError(Out.error(),
+ "failed to write to " + UniqueLockFileName);
sys::fs::remove(UniqueLockFileName);
// Don't call report_fatal_error.
Out.clear_error();
- return;
+ return std::move(Err);
}
}
@@ -221,23 +213,19 @@ LockFileManager::LockFileManager(StringRef FileName)
sys::fs::create_link(UniqueLockFileName, LockFileName);
if (!EC) {
RemoveUniqueFile.lockAcquired();
- return;
+ return true;
}
- if (EC != errc::file_exists) {
- std::string S("failed to create link ");
- raw_string_ostream OSS(S);
- OSS << LockFileName.str() << " to " << UniqueLockFileName.str();
- setError(EC, S);
- return;
- }
+ if (EC != errc::file_exists)
+ return createStringError(EC, "failed to create link " + LockFileName +
+ " to " + UniqueLockFileName);
// Someone else managed to create the lock file first. Read the process ID
// from the lock file.
if ((Owner = readLockFile(LockFileName))) {
// Wipe out our unique lock file (it's useless now)
sys::fs::remove(UniqueLockFileName);
- return;
+ return false;
}
if (!sys::fs::exists(LockFileName)) {
@@ -248,39 +236,14 @@ LockFileManager::LockFileManager(StringRef FileName)
// There is a lock file that nobody owns; try to clean it up and get
// ownership.
- if ((EC = sys::fs::remove(LockFileName))) {
- std::string S("failed to remove lockfile ");
- S.append(std::string(UniqueLockFileName));
- setError(EC, S);
- return;
- }
- }
-}
-
-LockFileManager::LockFileState LockFileManager::getState() const {
- if (Owner)
- return LFS_Shared;
-
- if (ErrorCode)
- return LFS_Error;
-
- return LFS_Owned;
-}
-
-std::string LockFileManager::getErrorMessage() const {
- if (ErrorCode) {
- std::string Str(ErrorDiagMsg);
- std::string ErrCodeMsg = ErrorCode.message();
- raw_string_ostream OSS(Str);
- if (!ErrCodeMsg.empty())
- OSS << ": " << ErrCodeMsg;
- return Str;
+ if ((EC = sys::fs::remove(LockFileName)))
+ return createStringError(EC, "failed to remove lockfile " +
+ UniqueLockFileName);
}
- return "";
}
LockFileManager::~LockFileManager() {
- if (getState() != LFS_Owned)
+ if (Owner)
return;
// Since we own the lock, remove the lock file and our own unique lock file.
@@ -293,7 +256,7 @@ LockFileManager::~LockFileManager() {
LockFileManager::WaitForUnlockResult
LockFileManager::waitForUnlock(const unsigned MaxSeconds) {
- if (getState() != LFS_Shared)
+ if (!Owner)
return Res_Success;
// Since we don't yet have an event-based method to wait for the lock file,
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
index 65aee2f70104b..65cbb13628301 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
@@ -1545,18 +1545,16 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
<< "'\n");
while (true) {
- llvm::LockFileManager Locked(LockFilePath.str());
- switch (Locked) {
- case LockFileManager::LFS_Error:
+ llvm::LockFileManager Lock(LockFilePath.str());
+ bool Owned;
+ if (Error Err = Lock.tryLock().moveInto(Owned)) {
+ consumeError(std::move(Err));
LLVM_DEBUG(
dbgs() << "[amdgpu-split-module] unable to acquire lockfile, debug "
"output may be mangled by other processes\n");
- Locked.unsafeRemoveLockFile();
- break;
- case LockFileManager::LFS_Owned:
- break;
- case LockFileManager::LFS_Shared: {
- switch (Locked.waitForUnlock()) {
+ Lock.unsafeRemoveLockFile();
+ } else if (!Owned) {
+ switch (Lock.waitForUnlock()) {
case LockFileManager::Res_Success:
break;
case LockFileManager::Res_OwnerDied:
@@ -1566,11 +1564,9 @@ PreservedAnalyses AMDGPUSplitModulePass::run(Module &M,
dbgs()
<< "[amdgpu-split-module] unable to acquire lockfile, debug "
"output may be mangled by other processes\n");
- Locked.unsafeRemoveLockFile();
+ Lock.unsafeRemoveLockFile();
break; // give up
}
- break;
- }
}
splitAMDGPUModule(TTIGetter, M, N, ModuleCallback);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jansvoboda11
added a commit
to jansvoboda11/llvm-project
that referenced
this pull request
Mar 10, 2025
This member will be needed after llvm#130627 anyway.
benlangmuir
reviewed
Mar 10, 2025
Bigcheese
approved these changes
Mar 11, 2025
benlangmuir
approved these changes
Mar 11, 2025
jansvoboda11
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 18, 2025
This patch removes some internal state out of `LockFileManager` by moving the locking code from the constructor into new member function `tryLock()` which returns the errors right away. This simplifies and modernizes the interface. (cherry picked from commit dafb566)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch removes some internal state out of
LockFileManagerby moving the locking code from the constructor into new member functiontryLock()which returns the errors right away. This simplifies and modernizes the interface.