[clang-tools-extra] r349348 - [clangd] Change diskbackedstorage to be atomic

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Mon Dec 17 04:38:22 2018
New Revision: 349348

URL: http://llvm.org/viewvc/llvm-project?rev=349348&view=rev
Log:
[clangd] Change diskbackedstorage to be atomic

Summary:
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.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, jfb, cfe-commits

Differential Revision: https://reviews.llvm.org/D55417

Modified:
    clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp

Modified: clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp?rev=349348&r1=349347&r2=349348&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp Mon Dec 17 04:38:22 2018
@@ -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"
@@ -33,6 +35,34 @@ std::string getShardPathFromFilePath(llv
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+                llvm::function_ref<void(llvm::raw_ostream &)> Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+      llvm::sys::fs::createUniqueFile(OutPath + ".tmp.%%%%%%%%", FD, TempPath);
+  if (EC)
+    return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+      llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+    return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+    return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@ public:
 
   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);
-    if (EC)
-      return llvm::errorCodeToError(EC);
-    OS << Shard;
-    OS.close();
-    return llvm::errorCodeToError(OS.error());
+    return writeAtomically(
+        getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+        [&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };