[sourcekit] Synchronize cursor-info requests on generated interfaces

Cursor info requires access to the underlying AST, which is not
thread-safe. This manifest as crashes when performing concurrent
cursor-info requests on the same generated interface. We already
prevented concurrent cursor-infos on regular Swift files by using the
ASTManager, but generated interfaces use the InterfaceGenContext which
may use either an ASTUnit or its own internal CompilerInstance.

rdar://problem/27311624

Author: benlangmuir

test/SourceKit/CursorInfo/cursor_info_async.swift

@@ -0,0 +1,26 @@
+import Foo
+
+// REQUIRES: objc_interop
+
+// Perform 8 concurrent cursor infos, which is often enough to cause
+// contention.  We disable printing the requests to minimize delay.
+
+// RUN: %sourcekitd-test -req=interface-gen-open -module Foo -- \
+// RUN:                  -F %S/../Inputs/libIDE-mock-sdk %mcp_opt %clang-importer-sdk \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 \
+// RUN:   == -async -dont-print-request -req=cursor -pos=60:15 | FileCheck %s -check-prefix=CHECK-FOO
+
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions
+// CHECK-FOO: <decl.struct><syntaxtype.keyword>struct</syntaxtype.keyword> <decl.name>FooRuncingOptions

tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp

@@ -209,6 +209,10 @@ namespace SourceKit {
   EditorDiagConsumer &ASTUnit::getEditorDiagConsumer() const {
     return Impl.CollectDiagConsumer;
   }
+
+  void ASTUnit::performAsync(std::function<void()> Fn) {
+    Impl.Queue.dispatch(std::move(Fn));
+  }
 }
 
 namespace {

tools/SourceKit/lib/SwiftLang/SwiftASTManager.h

@@ -17,6 +17,7 @@
 #include "SourceKit/Core/LLVM.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include <functional>
 #include <string>
 
 namespace llvm {
@@ -55,6 +56,10 @@ class ASTUnit : public SourceKit::ThreadSafeRefCountedBase<ASTUnit> {
   ArrayRef<ImmutableTextSnapshotRef> getSnapshots() const;
   EditorDiagConsumer &getEditorDiagConsumer() const;
   swift::SourceFile &getPrimarySourceFile() const;
+
+  /// Perform \p Fn asynchronously while preventing concurrent access to the
+  /// AST.
+  void performAsync(std::function<void()> Fn);
 };
 
 typedef IntrusiveRefCntPtr<ASTUnit> ASTUnitRef;

tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp

@@ -79,6 +79,10 @@ class SwiftInterfaceGenContext::Implementation {
   SourceTextInfo Info;
   // This is the non-typechecked AST for the generated interface source.
   CompilerInstance TextCI;
+  // Syncronize access to the embedded compiler instance (if we don't have an
+  // ASTUnit).
+  WorkQueue Queue{WorkQueue::Dequeuing::Serial,
+                  "sourcekit.swift.InterfaceGenContext"};
 };
 
 typedef SwiftInterfaceGenContext::Implementation::TextRange TextRange;
@@ -498,6 +502,14 @@ void SwiftInterfaceGenContext::reportEditorInfo(EditorConsumer &Consumer) const
   Consumer.finished();
 }
 
+void SwiftInterfaceGenContext::accessASTAsync(std::function<void()> Fn) {
+  if (Impl.AstUnit) {
+    Impl.AstUnit->performAsync(std::move(Fn));
+  } else {
+    Impl.Queue.dispatch(std::move(Fn));
+  }
+}
+
 SwiftInterfaceGenContext::ResolvedEntity
 SwiftInterfaceGenContext::resolveEntityForOffset(unsigned Offset) const {
   // Search among the references.
@@ -633,8 +645,10 @@ void SwiftLangSupport::editorOpenInterface(EditorConsumer &Consumer,
     return;
   }
 
-  IFaceGenContexts.set(Name, IFaceGenRef);
   IFaceGenRef->reportEditorInfo(Consumer);
+  // reportEditorInfo requires exclusive access to the AST, so don't add this
+  // to the service cache until it has returned.
+  IFaceGenContexts.set(Name, IFaceGenRef);
 }
 
 class PrimaryFileInterfaceConsumer : public SwiftASTConsumer {
@@ -740,8 +754,10 @@ void SwiftLangSupport::editorOpenHeaderInterface(EditorConsumer &Consumer,
     return;
   }
 
-  IFaceGenContexts.set(Name, IFaceGenRef);
   IFaceGenRef->reportEditorInfo(Consumer);
+  // reportEditorInfo requires exclusive access to the AST, so don't add this
+  // to the service cache until it has returned.
+  IFaceGenContexts.set(Name, IFaceGenRef);
 }
 
 void SwiftLangSupport::findInterfaceDocument(StringRef ModuleName,

tools/SourceKit/lib/SwiftLang/SwiftInterfaceGenContext.h

@@ -55,6 +55,7 @@ class SwiftInterfaceGenContext :
 
   bool matches(StringRef ModuleName, const swift::CompilerInvocation &Invok);
 
+  /// Note: requires exclusive access to the underlying AST.
   void reportEditorInfo(EditorConsumer &Consumer) const;
 
   struct ResolvedEntity {
@@ -71,8 +72,12 @@ class SwiftInterfaceGenContext :
     bool isResolved() const { return Dcl || Mod; }
   };
 
+  /// Provides exclusive access to the underlying AST.
+  void accessASTAsync(std::function<void()> Fn);
+
   /// Returns the resolved entity along with a boolean indicating if it is a
   /// reference or not.
+  /// Note: requires exclusive access to the underlying AST. See accessASTAsync.
   ResolvedEntity resolveEntityForOffset(unsigned Offset) const;
 
   /// Searches for a declaration with the given USR and returns the

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -1024,24 +1024,26 @@ void SwiftLangSupport::getCursorInfo(
                      SwiftArgs, OpArgs);
     }
 
-    SwiftInterfaceGenContext::ResolvedEntity Entity;
-    Entity = IFaceGenRef->resolveEntityForOffset(Offset);
-    if (Entity.isResolved()) {
-      CompilerInvocation Invok;
-      IFaceGenRef->applyTo(Invok);
-      if (Entity.Mod) {
-        passCursorInfoForModule(Entity.Mod, IFaceGenContexts, Invok, Receiver);
+    IFaceGenRef->accessASTAsync([this, IFaceGenRef, Offset, Receiver] {
+      SwiftInterfaceGenContext::ResolvedEntity Entity;
+      Entity = IFaceGenRef->resolveEntityForOffset(Offset);
+      if (Entity.isResolved()) {
+        CompilerInvocation Invok;
+        IFaceGenRef->applyTo(Invok);
+        if (Entity.Mod) {
+          passCursorInfoForModule(Entity.Mod, IFaceGenContexts, Invok,
+                                  Receiver);
+        } else {
+          // FIXME: Should pass the main module for the interface but currently
+          // it's not necessary.
+          passCursorInfoForDecl(
+              Entity.Dcl, /*MainModule*/ nullptr, Type(), Type(), Entity.IsRef,
+              /*OrigBufferID=*/None, *this, Invok, {}, Receiver);
+        }
       } else {
-        // FIXME: Should pass the main module for the interface but currently
-        // it's not necessary.
-        passCursorInfoForDecl(Entity.Dcl, /*MainModule*/nullptr, Type(), Type(),
-                              Entity.IsRef,
-                              /*OrigBufferID=*/None, *this, Invok,
-                              {}, Receiver);
+        Receiver({});
       }
-    } else {
-      Receiver({});
-    }
+    });
     return;
   }