[clang-tools-extra] r348147 - [clangd] Avoid memory-mapping files on Windows

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 3 07:21:49 PST 2018


Author: ibiryukov
Date: Mon Dec  3 07:21:49 2018
New Revision: 348147

URL: http://llvm.org/viewvc/llvm-project?rev=348147&view=rev
Log:
[clangd] Avoid memory-mapping files on Windows

Summary:
Memory-mapping files on Windows leads to them being locked and prevents
editors from saving changes to those files on disk. This is fine for the
compiler, but not acceptable for an interactive tool like clangd.
Therefore, we choose to avoid using memory-mapped files on Windows.

Reviewers: hokein, kadircet

Reviewed By: kadircet

Subscribers: yvvan, zturner, nik, malaperle, mgorny, ioeric, MaskRay, jkorous, arphaman, cfe-commits

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

Added:
    clang-tools-extra/trunk/clangd/FSProvider.cpp
Modified:
    clang-tools-extra/trunk/clangd/CMakeLists.txt
    clang-tools-extra/trunk/clangd/FSProvider.h

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=348147&r1=348146&r2=348147&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Mon Dec  3 07:21:49 2018
@@ -23,6 +23,7 @@ add_clang_library(clangDaemon
   FindSymbols.cpp
   FileDistance.cpp
   FS.cpp
+  FSProvider.cpp
   FuzzyMatch.cpp
   GlobalCompilationDatabase.cpp
   Headers.cpp

Added: clang-tools-extra/trunk/clangd/FSProvider.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.cpp?rev=348147&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/FSProvider.cpp (added)
+++ clang-tools-extra/trunk/clangd/FSProvider.cpp Mon Dec  3 07:21:49 2018
@@ -0,0 +1,85 @@
+//===--- FSProvider.cpp - VFS provider for ClangdServer -------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FSProvider.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include <memory>
+
+using namespace llvm;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+/// Always opens files in the underlying filesystem as "volatile", meaning they
+/// won't be memory-mapped. This avoid locking the files on Windows.
+class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
+public:
+  explicit VolatileFileSystem(llvm::IntrusiveRefCntPtr<FileSystem> FS)
+      : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr<std::unique_ptr<vfs::File>>
+  openFileForRead(const Twine &InPath) override {
+    SmallString<128> Path;
+    InPath.toVector(Path);
+
+    auto File = getUnderlyingFS().openFileForRead(Path);
+    if (!File)
+      return File;
+    // Try to guess preamble files, they can be memory-mapped even on Windows as
+    // clangd has exclusive access to those.
+    StringRef FileName = llvm::sys::path::filename(Path);
+    if (FileName.startswith("preamble-") && FileName.endswith(".pch"))
+      return File;
+    return std::unique_ptr<VolatileFile>(
+        new VolatileFile(std::move(*File)));
+  }
+
+private:
+  class VolatileFile : public vfs::File {
+  public:
+    VolatileFile(std::unique_ptr<vfs::File> Wrapped)
+        : Wrapped(std::move(Wrapped)) {
+      assert(this->Wrapped);
+    }
+
+    virtual llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
+    getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+              bool /*IsVolatile*/) override {
+      return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,
+                                /*IsVolatile=*/true);
+    }
+
+    llvm::ErrorOr<vfs::Status> status() override { return Wrapped->status(); }
+    llvm::ErrorOr<std::string> getName() override { return Wrapped->getName(); }
+    std::error_code close() override { return Wrapped->close(); }
+
+  private:
+    std::unique_ptr<File> Wrapped;
+  };
+};
+} // namespace
+
+llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
+clang::clangd::RealFileSystemProvider::getFileSystem() const {
+// Avoid using memory-mapped files on Windows, they cause file locking issues.
+// FIXME: Try to use a similar approach in Sema instead of relying on
+//        propagation of the 'isVolatile' flag through all layers.
+#ifdef _WIN32
+  return new VolatileFSProvider(vfs::getRealFileSystem());
+#else
+  return vfs::getRealFileSystem();
+#endif
+}
+} // namespace clangd
+} // namespace clang

Modified: clang-tools-extra/trunk/clangd/FSProvider.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FSProvider.h?rev=348147&r1=348146&r2=348147&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/FSProvider.h (original)
+++ clang-tools-extra/trunk/clangd/FSProvider.h Mon Dec  3 07:21:49 2018
@@ -33,9 +33,7 @@ class RealFileSystemProvider : public Fi
 public:
   // FIXME: returns the single real FS instance, which is not threadsafe.
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>
-  getFileSystem() const override {
-    return llvm::vfs::getRealFileSystem();
-  }
+  getFileSystem() const override;
 };
 
 } // namespace clangd




More information about the cfe-commits mailing list