Don’t crash SourceKit-LSP if swift-format crashes while we write the source file to stdin

When swift-format crashes before we send all data to its stdin, we get a SIGPIPE when trying to write more data to its studio on Linux. This, in turn, takes SourceKit-LSP down.

Do the same trick that we do when launching `JSONRPCConnection` of disabling SIGPIPE globally if we can’t disable it for individual pipes.

rdar://147665695

Author: ahoppen

Sources/LanguageServerProtocolJSONRPC/DisableSigpipe.swift

@@ -25,12 +25,20 @@ private let globallyIgnoredSIGPIPE: Bool = {
   _ = signal(SIGPIPE, SIG_IGN)
   return true
 }()
+#endif
 
-internal func globallyDisableSigpipe() {
+/// We receive a `SIGPIPE` if we write to a pipe that points to a crashed process. This in particular happens if the
+/// target of a `JSONRPCConnection` has crashed and we try to send it a message or if swift-format crashes and we try
+/// to send the source file to it.
+///
+/// On Darwin, `DispatchIO` and TSC `WritableByteStream` ignore `SIGPIPE` for the pipes handled by it, but that features
+/// is not available on Linux.
+/// Instead, globally ignore `SIGPIPE` on Linux to prevent us from crashing if the `JSONRPCConnection`'s target crashes.
+package func globallyDisableSigpipeIfNeeded() {
+  #if os(Linux) || os(Android)
   let haveWeIgnoredSIGPIEThisIsHereToTriggerIgnoringIt = globallyIgnoredSIGPIPE
   guard haveWeIgnoredSIGPIEThisIsHereToTriggerIgnoringIt else {
     fatalError("globallyIgnoredSIGPIPE should always be true")
   }
+  #endif
 }
-
-#endif

Sources/LanguageServerProtocolJSONRPC/JSONRPCConnection.swift

@@ -124,13 +124,7 @@ public final class JSONRPCConnection: Connection {
     self.inputMirrorFile = inputMirrorFile
     self.outputMirrorFile = outputMirrorFile
     self.receiveHandler = nil
-    #if os(Linux) || os(Android)
-    // We receive a `SIGPIPE` if we write to a pipe that points to a crashed process. This in particular happens if the
-    // target of a `JSONRPCConnection` has crashed and we try to send it a message.
-    // On Darwin, `DispatchIO` ignores `SIGPIPE` for the pipes handled by it, but that features is not available on Linux.
-    // Instead, globally ignore `SIGPIPE` on Linux to prevent us from crashing if the `JSONRPCConnection`'s target crashes.
-    globallyDisableSigpipe()
-    #endif
+    globallyDisableSigpipeIfNeeded()
     state = .created
     self.messageRegistry = messageRegistry
 

Sources/SourceKitLSP/Swift/DocumentFormatting.swift

@@ -13,6 +13,7 @@
 import Foundation
 package import LanguageServerProtocol
 import LanguageServerProtocolExtensions
+import LanguageServerProtocolJSONRPC
 import SKLogging
 import SKUtilities
 import SwiftExtensions
@@ -22,6 +23,7 @@ import TSCExtensions
 
 import struct TSCBasic.AbsolutePath
 import class TSCBasic.Process
+import protocol TSCBasic.WritableByteStream
 
 fileprivate extension String {
   init?(bytes: [UInt8], encoding: Encoding) {
@@ -189,12 +191,22 @@ extension SwiftLanguageService {
         "\(utf8Range.lowerBound):\(utf8UpperBound)",
       ]
     }
+    globallyDisableSigpipeIfNeeded()
     let process = TSCBasic.Process(arguments: args)
-    let writeStream = try process.launch()
+    let writeStream: any WritableByteStream
+    do {
+      writeStream = try process.launch()
+    } catch {
+      throw ResponseError.unknown("Launching swift-format failed: \(error)")
+    }
 
     // Send the file to format to swift-format's stdin. That way we don't have to write it to a file.
     writeStream.send(snapshot.text)
-    try writeStream.close()
+    do {
+      try writeStream.close()
+    } catch {
+      throw ResponseError.unknown("Writing to swift-format stdin failed: \(error)")
+    }
 
     let result = try await withTimeout(.seconds(60)) {
       try await process.waitUntilExitStoppingProcessOnTaskCancellation()

Tests/SourceKitLSPTests/FormattingTests.swift

@@ -14,8 +14,12 @@ import LanguageServerProtocol
 import SKLogging
 import SKTestSupport
 import SourceKitLSP
+import SwiftExtensions
+import ToolchainRegistry
 import XCTest
 
+import class TSCBasic.Process
+
 final class FormattingTests: XCTestCase {
   func testFormatting() async throws {
     try await SkipUnless.swiftFormatSupportsDashToIndicateReadingFromStdin()
@@ -306,4 +310,57 @@ final class FormattingTests: XCTestCase {
       ]
     )
   }
+
+  func testSwiftFormatCrashing() async throws {
+    try await withTestScratchDir { scratchDir in
+      let toolchain = try await unwrap(ToolchainRegistry.forTesting.default)
+
+      let crashingSwiftFilePath = scratchDir.appendingPathComponent("crashing-executable.swift")
+      let crashingExecutablePath = scratchDir.appendingPathComponent("crashing-executable")
+      try await "fatalError()".writeWithRetry(to: crashingSwiftFilePath)
+      var compilerArguments = try [
+        crashingSwiftFilePath.filePath,
+        "-o",
+        crashingExecutablePath.filePath,
+      ]
+      if let defaultSDKPath {
+        compilerArguments += ["-sdk", defaultSDKPath]
+      }
+      try await Process.checkNonZeroExit(arguments: [XCTUnwrap(toolchain.swiftc?.filePath)] + compilerArguments)
+
+      let toolchainRegistry = ToolchainRegistry(toolchains: [
+        Toolchain(
+          identifier: "\(toolchain.identifier)-crashing-swift-format",
+          displayName: "\(toolchain.identifier) with crashing swift-format",
+          path: toolchain.path,
+          clang: toolchain.clang,
+          swift: toolchain.swift,
+          swiftc: toolchain.swiftc,
+          swiftFormat: crashingExecutablePath,
+          clangd: toolchain.clangd,
+          sourcekitd: toolchain.sourcekitd,
+          sourceKitClientPlugin: toolchain.sourceKitClientPlugin,
+          sourceKitServicePlugin: toolchain.sourceKitServicePlugin,
+          libIndexStore: toolchain.libIndexStore
+        )
+      ])
+      let testClient = try await TestSourceKitLSPClient(toolchainRegistry: toolchainRegistry)
+      let uri = DocumentURI(for: .swift)
+      testClient.openDocument(
+        // Generate a large source file to increase the chance of the executable we substitute for swift-format
+        // crashing before the entire input file is sent to it.
+        String(repeating: "func foo() {}\n", count: 10_000),
+        uri: uri
+      )
+      await assertThrowsError(
+        try await testClient.send(
+          DocumentFormattingRequest(
+            textDocument: TextDocumentIdentifier(uri),
+            options: FormattingOptions(tabSize: 2, insertSpaces: true)
+          )
+        ),
+        expectedMessage: #/Running swift-format failed|Writing to swift-format stdin failed/#
+      )
+    }
+  }
 }