Fix a race condition that could cause the build graph to not be generated when doing initial background indexing

We were making the initial `generateBuildGraph` call in `SourceKitLSPServer` from a `Task`. This means that `generateBuildGraph` could be executed after `waitForUpToDateBuildGraph` was called by `SemanticIndexManager`. Thus `waitForUpToDateBuildGraph` returned immediately and no files were background indexed.

Make `generateBuildGraph` immediately schedule a package reload for SwiftPM build systems instead of doing the hop through a `Task`, fixing the race condition.

rdar://135551812

Author: ahoppen

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -281,7 +281,7 @@ extension BuildServerBuildSystem: BuildSystem {
     return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
   }
 
-  package func generateBuildGraph() {}
+  package func scheduleBuildGraphGeneration() {}
 
   package func waitForUpToDateBuildGraph() async {}
 

Sources/BuildSystemIntegration/BuildSystem.swift

@@ -137,8 +137,10 @@ package protocol BuildSystem: AnyObject, Sendable {
   /// Return the list of targets and run destinations that the given document can be built for.
   func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]
 
-  /// Re-generate the build graph.
-  func generateBuildGraph() async throws
+  /// Schedule a task that re-generates the build graph. The function may return before the build graph has finished
+  /// being generated. If clients need to wait for an up-to-date build graph, they should call
+  /// `waitForUpToDateBuildGraph` afterwards.
+  func scheduleBuildGraphGeneration() async throws
 
   /// Wait until the build graph has been loaded.
   func waitForUpToDateBuildGraph() async

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -265,8 +265,8 @@ extension BuildSystemManager {
     return settings
   }
 
-  package func generateBuildGraph() async throws {
-    try await self.buildSystem?.generateBuildGraph()
+  package func scheduleBuildGraphGeneration() async throws {
+    try await self.buildSystem?.scheduleBuildGraphGeneration()
   }
 
   package func waitForUpToDateBuildGraph() async {

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -133,7 +133,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     throw PrepareNotSupportedError()
   }
 
-  package func generateBuildGraph() {}
+  package func scheduleBuildGraphGeneration() {}
 
   package func waitForUpToDateBuildGraph() async {}
 

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -365,10 +365,11 @@ package actor SwiftPMBuildSystem {
 extension SwiftPMBuildSystem {
   /// (Re-)load the package settings by parsing the manifest and resolving all the targets and
   /// dependencies.
-  package func reloadPackage() async throws {
-    try await packageLoadingQueue.asyncThrowing {
+  @discardableResult
+  package func schedulePackageReload() -> Task<Void, Swift.Error> {
+    return packageLoadingQueue.asyncThrowing {
       try await self.reloadPackageImpl()
-    }.valuePropagatingCancellation
+    }
   }
 
   /// - Important: Must only be called on `packageLoadingQueue`.
@@ -549,8 +550,8 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
     return []
   }
 
-  package func generateBuildGraph() async throws {
-    try await self.reloadPackage()
+  package func scheduleBuildGraphGeneration() async throws {
+    self.schedulePackageReload()
   }
 
   package func waitForUpToDateBuildGraph() async {
@@ -743,7 +744,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
     if events.contains(where: { self.fileEventShouldTriggerPackageReload(event: $0) }) {
       logger.log("Reloading package because of file change")
       await orLog("Reloading package") {
-        try await self.reloadPackage()
+        try await self.schedulePackageReload().value
       }
     }
 

Sources/SourceKitLSP/SourceKitLSPServer.swift

@@ -967,13 +967,11 @@ extension SourceKitLSPServer {
     guard await condition(buildSystem) else {
       return nil
     }
-    Task {
-      await orLog("Initial build graph generation") {
-        // Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
-        // call `fileHandlingCapabilityChanged`, which allows us to move documents to a workspace with this build
-        // system.
-        try await buildSystem?.generateBuildGraph()
-      }
+    await orLog("Initial build graph generation") {
+      // Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
+      // call `fileHandlingCapabilityChanged`, which allows us to move documents to a workspace with this build
+      // system.
+      try await buildSystem?.scheduleBuildGraphGeneration()
     }
 
     let projectRoot = await buildSystem?.projectRoot.pathString

Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift

@@ -482,7 +482,7 @@ class ManualBuildSystem: BuildSystem {
     throw PrepareNotSupportedError()
   }
 
-  package func generateBuildGraph() {}
+  package func scheduleBuildGraphGeneration() {}
 
   package func waitForUpToDateBuildGraph() async {}
 

Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift

@@ -83,7 +83,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      await assertThrowsError(try await buildSystem.generateBuildGraph())
+      await assertThrowsError(try await buildSystem.schedulePackageReload().value)
     }
   }
 
@@ -144,7 +144,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
@@ -209,7 +209,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aPlusSomething = packageRoot.appending(components: "Sources", "lib", "a+something.swift")
       let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
@@ -272,7 +272,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(swiftPM: options),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
@@ -354,7 +354,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let source = try resolveSymlinks(packageRoot.appending(component: "Package.swift"))
       let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: source.asURI, language: .swift))
@@ -391,7 +391,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let bswift = packageRoot.appending(components: "Sources", "lib", "b.swift")
@@ -440,7 +440,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift")
       let bswift = packageRoot.appending(components: "Sources", "libB", "b.swift")
@@ -495,7 +495,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift")
       let bswift = packageRoot.appending(components: "Sources", "libB", "b.swift")
@@ -539,7 +539,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let acxx = packageRoot.appending(components: "Sources", "lib", "a.cpp")
       let bcxx = packageRoot.appending(components: "Sources", "lib", "b.cpp")
@@ -620,7 +620,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: aswift.asURI, language: .swift))
@@ -672,7 +672,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift1 = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let aswift2 =
@@ -740,7 +740,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       for file in [acpp, ah] {
         let args = try unwrap(
@@ -780,7 +780,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
       let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: aswift.asURI, language: .swift))
@@ -856,7 +856,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
         options: SourceKitLSPOptions(),
         testHooks: SwiftPMTestHooks()
       )
-      try await swiftpmBuildSystem.generateBuildGraph()
+      try await swiftpmBuildSystem.schedulePackageReload().value
 
       let aswift = packageRoot.appending(components: "Plugins", "MyPlugin", "a.swift")
       let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple

Tests/SourceKitLSPTests/BuildSystemTests.swift

@@ -73,7 +73,7 @@ actor TestBuildSystem: BuildSystem {
     throw PrepareNotSupportedError()
   }
 
-  func generateBuildGraph() {}
+  func scheduleBuildGraphGeneration() {}
 
   package func waitForUpToDateBuildGraph() async {}