[clang-tools-extra] r296737 - [clangd] Fix a potential race by copying the FixIts out of ASTManager before releasing the lock.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Thu Mar  2 04:25:00 2017
New Revision: 296737

URL: http://llvm.org/viewvc/llvm-project?rev=296737&view=rev
Log:
[clangd] Fix a potential race by copying the FixIts out of ASTManager before releasing the lock.

Also document the locking semantics a bit better.

Modified:
    clang-tools-extra/trunk/clangd/ASTManager.cpp
    clang-tools-extra/trunk/clangd/ASTManager.h
    clang-tools-extra/trunk/clangd/DocumentStore.h

Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.cpp?rev=296737&r1=296736&r2=296737&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ASTManager.cpp (original)
+++ clang-tools-extra/trunk/clangd/ASTManager.cpp Thu Mar  2 04:25:00 2017
@@ -224,11 +224,11 @@ ASTManager::createASTUnitForFile(StringR
       /*AllowPCHWithCompilerErrors=*/true));
 }
 
-llvm::ArrayRef<clang::tooling::Replacement>
+std::vector<clang::tooling::Replacement>
 ASTManager::getFixIts(const clangd::Diagnostic &D) {
   std::lock_guard<std::mutex> Guard(FixItLock);
   auto I = FixIts.find(D);
   if (I != FixIts.end())
     return I->second;
-  return llvm::None;
+  return {};
 }

Modified: clang-tools-extra/trunk/clangd/ASTManager.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.h?rev=296737&r1=296736&r2=296737&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ASTManager.h (original)
+++ clang-tools-extra/trunk/clangd/ASTManager.h Thu Mar  2 04:25:00 2017
@@ -39,7 +39,10 @@ public:
   // FIXME: Implement codeComplete
 
   /// Get the fixes associated with a certain diagnostic as replacements.
-  llvm::ArrayRef<clang::tooling::Replacement>
+  ///
+  /// This function is thread-safe. It returns a copy to avoid handing out
+  /// references to unguarded data.
+  std::vector<clang::tooling::Replacement>
   getFixIts(const clangd::Diagnostic &D);
 
   DocumentStore &getStore() const { return Store; }

Modified: clang-tools-extra/trunk/clangd/DocumentStore.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DocumentStore.h?rev=296737&r1=296736&r2=296737&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/DocumentStore.h (original)
+++ clang-tools-extra/trunk/clangd/DocumentStore.h Thu Mar  2 04:25:00 2017
@@ -49,6 +49,9 @@ public:
       Listener->onDocumentRemove(Uri);
   }
   /// Retrieve a document from the store. Empty string if it's unknown.
+  ///
+  /// This function is thread-safe. It returns a copy to avoid handing out
+  /// references to unguarded data.
   std::string getDocument(StringRef Uri) const {
     // FIXME: This could be a reader lock.
     std::lock_guard<std::mutex> Guard(DocsMutex);
@@ -59,6 +62,9 @@ public:
   void addListener(DocumentStoreListener *DSL) { Listeners.push_back(DSL); }
 
   /// Get name and constents of all documents in this store.
+  ///
+  /// This function is thread-safe. It returns a copies to avoid handing out
+  /// references to unguarded data.
   std::vector<std::pair<std::string, std::string>> getAllDocuments() const {
     std::vector<std::pair<std::string, std::string>> AllDocs;
     std::lock_guard<std::mutex> Guard(DocsMutex);