kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay, ioeric.
There was a chance that multiple clangd instances could try to write
same shard, in which case we would get a malformed file most likely. This patch
changes the writing mechanism to first write to a temporary file and then rename
it to fit real destination. Which is guaranteed to be atomic by POSIX.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D55417
Files:
clangd/index/BackgroundIndexStorage.cpp
Index: clangd/index/BackgroundIndexStorage.cpp
===================================================================
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
#include "Logger.h"
#include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -70,13 +72,28 @@
llvm::Error storeShard(llvm::StringRef ShardIdentifier,
IndexFileOut Shard) const override {
- auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
- std::error_code EC;
- llvm::raw_fd_ostream OS(ShardPath, EC);
+ // Write to a temporary file first.
+ llvm::SmallString<64> TempPath;
+ int FD;
+ auto EC = llvm::sys::fs::createTemporaryFile("clangd", "index-shard", FD,
+ TempPath);
if (EC)
return llvm::errorCodeToError(EC);
+ // Make sure temp file is destroyed at exit.
+ auto _ =
+ llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+ llvm::raw_fd_ostream OS(FD, true);
OS << Shard;
OS.close();
+ if (OS.has_error())
+ return llvm::errorCodeToError(OS.error());
+
+ // Then move to real location.
+ const auto ShardPath =
+ getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+ EC = llvm::sys::fs::rename(TempPath, ShardPath);
+ if (EC)
+ return llvm::errorCodeToError(OS.error());
return llvm::errorCodeToError(OS.error());
}
};
Index: clangd/index/BackgroundIndexStorage.cpp
===================================================================
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
#include "Logger.h"
#include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -70,13 +72,28 @@
llvm::Error storeShard(llvm::StringRef ShardIdentifier,
IndexFileOut Shard) const override {
- auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
- std::error_code EC;
- llvm::raw_fd_ostream OS(ShardPath, EC);
+ // Write to a temporary file first.
+ llvm::SmallString<64> TempPath;
+ int FD;
+ auto EC = llvm::sys::fs::createTemporaryFile("clangd", "index-shard", FD,
+ TempPath);
if (EC)
return llvm::errorCodeToError(EC);
+ // Make sure temp file is destroyed at exit.
+ auto _ =
+ llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+ llvm::raw_fd_ostream OS(FD, true);
OS << Shard;
OS.close();
+ if (OS.has_error())
+ return llvm::errorCodeToError(OS.error());
+
+ // Then move to real location.
+ const auto ShardPath =
+ getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
+ EC = llvm::sys::fs::rename(TempPath, ShardPath);
+ if (EC)
+ return llvm::errorCodeToError(OS.error());
return llvm::errorCodeToError(OS.error());
}
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
