Merge pull request #366 from ahoppen/clangd-crash-recovery

Re-enable clang crash recovery tests

Author: ahoppen

Sources/SourceKitLSP/Clang/ClangLanguageServer.swift

@@ -75,6 +75,9 @@ final class ClangLanguageServerShim: LanguageServer, ToolchainLanguageServer {
   /// A callback with which `ClangLanguageServer` can request its owner to reopen all documents in case it has crashed.
   private let reopenDocuments: (ToolchainLanguageServer) -> Void
 
+  /// While `clangd` is running, its PID.
+  private var clangdPid: Int32?
+
   /// Creates a language server for the given client referencing the clang binary at the given path.
   public init(
     client: Connection,
@@ -138,12 +141,12 @@ final class ClangLanguageServerShim: LanguageServer, ToolchainLanguageServer {
     process.terminationHandler = { [weak self] process in
       log("clangd exited: \(process.terminationReason) \(process.terminationStatus)")
       connectionToClangd.close()
-      if process.terminationStatus != 0 {
-        if let self = self {
-          self.queue.async {
-            self.state = .connectionInterrupted
-            self.restartClangd()
-          }
+      guard let self = self else { return }
+      self.queue.async {
+        self.clangdPid = nil
+        if process.terminationStatus != 0 {
+          self.state = .connectionInterrupted
+          self.restartClangd()
         }
       }
     }
@@ -153,6 +156,7 @@ final class ClangLanguageServerShim: LanguageServer, ToolchainLanguageServer {
     } else {
       process.launch()
     }
+    self.clangdPid = process.processIdentifier
   }
 
   /// Restart `clangd` after it has crashed.
@@ -249,6 +253,15 @@ final class ClangLanguageServerShim: LanguageServer, ToolchainLanguageServer {
       self.clangd.send(notification)
     }
   }
+
+  func _crash() {
+    self.queue.async {
+      // Since `clangd` doesn't have a method to crash it, kill it.
+      if let pid = self.clangdPid {
+        kill(pid, SIGKILL)
+      }
+    }
+  }
 
   /// Forward the given `request` to `clangd` by asynchronously switching to `queue` for a thread-safe access to `clangd`.
   private func forwardRequestToClangdOnQueue<R>(_ request: Request<R>, _ handler: ((LSPResult<R.Response>) -> Void)? = nil) {

Sources/SourceKitLSP/ToolchainLanguageServer.swift

@@ -79,4 +79,7 @@ public protocol ToolchainLanguageServer: AnyObject {
   // MARK: - Other
 
   func executeCommand(_ req: Request<ExecuteCommandRequest>)
+
+  /// Crash the language server. Should be used for crash recovery testing only.
+  func _crash()
 }

Tests/SourceKitDTests/CrashRecoveryTests.swift

@@ -108,9 +108,9 @@ final class CrashRecoveryTests: XCTestCase {
   /// Crashes clangd and waits for it to restart
   /// - Parameters:
   ///   - ws: The workspace for which the clangd server shall be crashed
-  ///   - loc: A test location that points to the first line of a given file. This line needs to be otherwise empty.
-  private func crashClangd(for ws: SKTibsTestWorkspace, firstLineLoc loc: TestLocation) {
-    let clangdServer = ws.testServer.server!._languageService(for: loc.docUri, .cpp, in: ws.testServer.server!.workspace!)!
+  ///   - document: The URI of a C/C++/... document in the workspace
+  private func crashClangd(for ws: SKTibsTestWorkspace, document docUri: DocumentURI) {
+    let clangdServer = ws.testServer.server!._languageService(for: docUri, .cpp, in: ws.testServer.server!.workspace!)!
 
     let clangdCrashed = self.expectation(description: "clangd crashed")
     let clangdRestarted = self.expectation(description: "clangd restarted")
@@ -126,21 +126,13 @@ final class CrashRecoveryTests: XCTestCase {
       }
     }
 
-    // Add a pragma to crash clang
-    let addCrashPragma = TextDocumentContentChangeEvent(range: loc.position..<loc.position, rangeLength: 0, text: "#pragma clang __debug crash\n")
-    ws.sk.send(DidChangeTextDocumentNotification(textDocument: VersionedTextDocumentIdentifier(loc.docUri, version: 3), contentChanges: [addCrashPragma]))
+    clangdServer._crash()
 
     self.wait(for: [clangdCrashed], timeout: 5)
-
-    // Once clangds has crashed, remove the pragma again to allow it to restart
-    let removeCrashPragma = TextDocumentContentChangeEvent(range: loc.position..<Position(line: 1, utf16index: 0), rangeLength: 28, text: "")
-    ws.sk.send(DidChangeTextDocumentNotification(textDocument: VersionedTextDocumentIdentifier(loc.docUri, version: 4), contentChanges: [removeCrashPragma]))
-
     self.wait(for: [clangdRestarted], timeout: 30)
   }
 
   func testClangdCrashRecovery() throws {
-    throw XCTSkip("failing on rebranch - rdar://73717447")
     try XCTSkipUnless(longTestsEnabled)
 
     let ws = try! staticSourceKitTibsWorkspace(name: "ClangCrashRecovery")!
@@ -167,7 +159,7 @@ final class CrashRecoveryTests: XCTestCase {
 
     // Crash clangd
 
-    crashClangd(for: ws, firstLineLoc: loc)
+    crashClangd(for: ws, document: loc.docUri)
 
     // Check that we have re-opened the document with the correct in-memory state
 
@@ -178,7 +170,6 @@ final class CrashRecoveryTests: XCTestCase {
   }
 
   func testClangdCrashRecoveryReopensWithCorrectBuildSettings() throws {
-    throw XCTSkip("failing on rebranch - rdar://73717447")
     try XCTSkipUnless(longTestsEnabled)
 
     let ws = try! staticSourceKitTibsWorkspace(name: "ClangCrashRecoveryBuildSettings")!
@@ -199,7 +190,7 @@ final class CrashRecoveryTests: XCTestCase {
 
     // Crash clangd
 
-    crashClangd(for: ws, firstLineLoc: loc)
+    crashClangd(for: ws, document: loc.docUri)
 
     // Check that we have re-opened the document with the correct build settings
     // If we did not recover the correct build settings, document highlight would
@@ -212,7 +203,6 @@ final class CrashRecoveryTests: XCTestCase {
   }
 
   func testPreventClangdCrashLoop() throws {
-    throw XCTSkip("failing on rebranch - rdar://73717447")
     try XCTSkipUnless(longTestsEnabled)
 
     let ws = try! staticSourceKitTibsWorkspace(name: "ClangCrashRecovery")!
@@ -231,53 +221,38 @@ final class CrashRecoveryTests: XCTestCase {
 
     let clangdCrashed = self.expectation(description: "clangd crashed")
     clangdCrashed.assertForOverFulfill = false
-    // assertForOverFulfill is not working on Linux (SR-12575). Manually keep track if we have already called fulfill on the expectation
-    var clangdCrashedFulfilled = false
 
     let clangdRestartedFirstTime = self.expectation(description: "clangd restarted for the first time")
-    
     let clangdRestartedSecondTime = self.expectation(description: "clangd restarted for the second time")
-    clangdRestartedSecondTime.assertForOverFulfill = false
-    // assertForOverFulfill is not working on Linux (SR-12575). Manually keep track if we have already called fulfill on the expectation
-    var clangdRestartedSecondTimeFulfilled = false
-    
+
     var clangdHasRestartedFirstTime = false
 
     clangdServer.addStateChangeHandler { (oldState, newState) in
       switch newState {
       case .connectionInterrupted:
-        if !clangdCrashedFulfilled {
-          clangdCrashed.fulfill()
-          clangdCrashedFulfilled = true
-        }
+        clangdCrashed.fulfill()
       case .connected:
         if !clangdHasRestartedFirstTime {
           clangdRestartedFirstTime.fulfill()
           clangdHasRestartedFirstTime = true
         } else {
-          if !clangdRestartedSecondTimeFulfilled {
-            clangdRestartedSecondTime.fulfill()
-            clangdRestartedSecondTimeFulfilled = true
-          }
+          clangdRestartedSecondTime.fulfill()
         }
       default:
         break
       }
     }
 
-    // Add a pragma to crash clang
-    
-    let addCrashPragma = TextDocumentContentChangeEvent(range: loc.position..<loc.position, rangeLength: 0, text: "#pragma clang __debug crash\n")
-    ws.sk.send(DidChangeTextDocumentNotification(textDocument: VersionedTextDocumentIdentifier(loc.docUri, version: 3), contentChanges: [addCrashPragma]))
+    clangdServer._crash()
 
     self.wait(for: [clangdCrashed], timeout: 5)
-
-    // Clangd has crashed for the first time. Leave the pragma there to crash clangd once again.
-    
     self.wait(for: [clangdRestartedFirstTime], timeout: 30)
     // Clangd has restarted. Note the date so we can check that the second restart doesn't happen too quickly.
     let firstRestartDate = Date()
-    
+
+    // Crash clangd again. This time, it should only restart after a delay.
+    clangdServer._crash()
+
     self.wait(for: [clangdRestartedSecondTime], timeout: 30)
     XCTAssert(Date().timeIntervalSince(firstRestartDate) > 5, "Clangd restarted too quickly after crashing twice in a row. We are not preventing crash loops.")
   }