[PATCH] D55417: [clangd] Change diskbackedstorage to be atomic

[Kadir Cetinkaya via Phabricator via cfe-commits]

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());
   }
 };


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55417.177145.patch
Type: text/x-patch
Size: 1631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181207/51dc1ea6/attachment.bin>