From 8c25a9713304bee822ff44a85d1ae2095787d9fc Mon Sep 17 00:00:00 2001 From: rudy Date: Tue, 3 May 2022 10:57:48 +0200 Subject: [PATCH] fix(KeySetCache): remove a race condition thanks to Umut for pointing it --- compiler/lib/ClientLib/KeySetCache.cpp | 34 ++++++++++++-------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/compiler/lib/ClientLib/KeySetCache.cpp b/compiler/lib/ClientLib/KeySetCache.cpp index 96f3e0d16..9f911ab84 100644 --- a/compiler/lib/ClientLib/KeySetCache.cpp +++ b/compiler/lib/ClientLib/KeySetCache.cpp @@ -197,18 +197,6 @@ KeySetCache::loadOrGenerateSave(ClientParameters ¶ms, uint64_t seed_msb, llvm::sys::path::append(folderPath, std::to_string(seed_msb) + "_" + std::to_string(seed_lsb)); - if (llvm::sys::fs::exists(folderPath)) { - auto keys = loadKeys(params, seed_msb, seed_lsb, std::string(folderPath)); - if (keys.has_value()) { - return keys; - } else { - std::cerr << std::string(keys.error().mesg) << "\n"; - std::cerr << "Regenerating KeySetCache entry " << std::string(folderPath) - << "\n"; - llvm::sys::fs::remove_directories(folderPath); - } - } - // Creating a lock for concurrent generation llvm::SmallString<0> lockPath(folderPath); lockPath.append("lock"); @@ -225,19 +213,29 @@ KeySetCache::loadOrGenerateSave(ClientParameters ¶ms, uint64_t seed_msb, << std::string(lockPath) << "\": " << err.message(); } - // The first to lock will generate while the others waits - llvm::sys::fs::lockFile(FD_lock); - auto cleanUnlock = llvm::make_scope_exit([&]() { + // The lock is released when the function returns. + // => any intermediate state in the function is not visible to others. + auto unlockAtReturn = llvm::make_scope_exit([&]() { llvm::sys::fs::unlockFile(FD_lock); llvm::sys::fs::remove(lockPath); }); + llvm::sys::fs::lockFile(FD_lock); if (llvm::sys::fs::exists(folderPath)) { - // Others returns here - return loadKeys(params, seed_msb, seed_lsb, std::string(folderPath)); + // Once it has been generated by another process (or was alread here) + auto keys = loadKeys(params, seed_msb, seed_lsb, std::string(folderPath)); + if (keys.has_value()) { + return keys; + } else { + std::cerr << std::string(keys.error().mesg) << "\n"; + std::cerr << "Invalid KeySetCache entry " << std::string(folderPath) + << "\n"; + llvm::sys::fs::remove_directories(folderPath); + // Then we can continue as it didn't exist + } } - std::cerr << "KeySetCache: miss, regenerating \n"; + std::cerr << "KeySetCache: miss, regenerating\n"; OUTCOME_TRY(auto key_set, KeySet::generate(params, seed_msb, seed_lsb)); OUTCOME_TRYV(saveKeys(*key_set, folderPath));