Skip to content

[6.0] Send SIGKILL to swift-frontend indexing processes #1525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions Sources/SKSupport/Process+Run.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,30 @@ import WinSDK

extension Process {
/// Wait for the process to exit. If the task gets cancelled, during this time, send a `SIGINT` to the process.
/// Should the process not terminate on SIGINT after 2 seconds, it is killed using `SIGKILL`.
@discardableResult
public func waitUntilExitSendingSigIntOnTaskCancellation() async throws -> ProcessResult {
public func waitUntilExitStoppingProcessOnTaskCancellation() async throws -> ProcessResult {
let hasExited = AtomicBool(initialValue: false)
return try await withTaskCancellationHandler {
try await waitUntilExit()
defer {
hasExited.value = true
}
return try await waitUntilExit()
} onCancel: {
signal(SIGINT)
Task {
// Give the process 2 seconds to react to a SIGINT. If that doesn't work, kill the process.
try await Task.sleep(for: .seconds(2))
if !hasExited.value {
#if os(Windows)
// Windows does not define SIGKILL. Process.signal sends a `terminate` to the underlying Foundation process
// for any signal that is not SIGINT. Use `SIGABRT` to terminate the process.
signal(SIGABRT)
#else
signal(SIGKILL)
#endif
}
}
}
}

Expand Down Expand Up @@ -138,7 +156,7 @@ extension Process {
)
return try await withTaskPriorityChangedHandler(initialPriority: Task.currentPriority) { @Sendable in
setProcessPriority(pid: process.processID, newPriority: Task.currentPriority)
return try await process.waitUntilExitSendingSigIntOnTaskCancellation()
return try await process.waitUntilExitStoppingProcessOnTaskCancellation()
} taskPriorityChanged: {
setProcessPriority(pid: process.processID, newPriority: Task.currentPriority)
}
Expand Down
8 changes: 8 additions & 0 deletions Sources/SemanticIndex/SemanticIndexManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ public final actor SemanticIndexManager {
/// The build system manager that is used to get compiler arguments for a file.
private let buildSystemManager: BuildSystemManager

/// How long to wait until we cancel an update indexstore task. This timeout should be long enough that all
/// `swift-frontend` tasks finish within it. It prevents us from blocking the index if the type checker gets stuck on
/// an expression for a long time.
public var updateIndexStoreTimeout: Duration

private let testHooks: IndexTestHooks

/// The task to generate the build graph (resolving package dependencies, generating the build description,
Expand Down Expand Up @@ -209,6 +214,7 @@ public final actor SemanticIndexManager {
public init(
index: UncheckedIndex,
buildSystemManager: BuildSystemManager,
updateIndexStoreTimeout: Duration,
testHooks: IndexTestHooks,
indexTaskScheduler: TaskScheduler<AnyIndexTaskDescription>,
logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void,
Expand All @@ -217,6 +223,7 @@ public final actor SemanticIndexManager {
) {
self.index = index
self.buildSystemManager = buildSystemManager
self.updateIndexStoreTimeout = updateIndexStoreTimeout
self.testHooks = testHooks
self.indexTaskScheduler = indexTaskScheduler
self.logMessageToIndexLog = logMessageToIndexLog
Expand Down Expand Up @@ -510,6 +517,7 @@ public final actor SemanticIndexManager {
index: index,
indexStoreUpToDateTracker: indexStoreUpToDateTracker,
logMessageToIndexLog: logMessageToIndexLog,
timeout: updateIndexStoreTimeout,
testHooks: testHooks
)
)
Expand Down
34 changes: 22 additions & 12 deletions Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import SwiftExtensions

import struct TSCBasic.AbsolutePath
import class TSCBasic.Process
import struct TSCBasic.ProcessResult

private let updateIndexStoreIDForLogging = AtomicUInt32(initialValue: 1)

Expand Down Expand Up @@ -114,6 +115,11 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
/// See `SemanticIndexManager.logMessageToIndexLog`.
private let logMessageToIndexLog: @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void

/// How long to wait until we cancel an update indexstore task. This timeout should be long enough that all
/// `swift-frontend` tasks finish within it. It prevents us from blocking the index if the type checker gets stuck on
/// an expression for a long time.
private let timeout: Duration

/// Test hooks that should be called when the index task finishes.
private let testHooks: IndexTestHooks

Expand All @@ -140,13 +146,15 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
index: UncheckedIndex,
indexStoreUpToDateTracker: UpToDateTracker<DocumentURI>,
logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void,
timeout: Duration,
testHooks: IndexTestHooks
) {
self.filesToIndex = filesToIndex
self.buildSystemManager = buildSystemManager
self.index = index
self.indexStoreUpToDateTracker = indexStoreUpToDateTracker
self.logMessageToIndexLog = logMessageToIndexLog
self.timeout = timeout
self.testHooks = testHooks
}

Expand Down Expand Up @@ -356,19 +364,21 @@ public struct UpdateIndexStoreTaskDescription: IndexTaskDescription {
let stdoutHandler = PipeAsStringHandler { logMessageToIndexLog(logID, $0) }
let stderrHandler = PipeAsStringHandler { logMessageToIndexLog(logID, $0) }

// Time out updating of the index store after 2 minutes. We don't expect any single file compilation to take longer
// than 2 minutes in practice, so this indicates that the compiler has entered a loop and we probably won't make any
// progress here. We will try indexing the file again when it is edited or when the project is re-opened.
// 2 minutes have been chosen arbitrarily.
let result = try await withTimeout(.seconds(120)) {
try await Process.run(
arguments: processArguments,
workingDirectory: workingDirectory,
outputRedirection: .stream(
stdout: { stdoutHandler.handleDataFromPipe(Data($0)) },
stderr: { stderrHandler.handleDataFromPipe(Data($0)) }
let result: ProcessResult
do {
result = try await withTimeout(timeout) {
try await Process.run(
arguments: processArguments,
workingDirectory: workingDirectory,
outputRedirection: .stream(
stdout: { stdoutHandler.handleDataFromPipe(Data($0)) },
stderr: { stderrHandler.handleDataFromPipe(Data($0)) }
)
)
)
}
} catch {
logMessageToIndexLog(logID, "Finished error in \(start.duration(to: .now)): \(error)")
throw error
}
let exitStatus = result.exitStatus.exhaustivelySwitchable
logMessageToIndexLog(logID, "Finished with \(exitStatus.description) in \(start.duration(to: .now))")
Expand Down
2 changes: 1 addition & 1 deletion Sources/SourceKitLSP/Swift/DocumentFormatting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ extension SwiftLanguageService {
writeStream.send(snapshot.text)
try writeStream.close()

let result = try await process.waitUntilExitSendingSigIntOnTaskCancellation()
let result = try await process.waitUntilExitStoppingProcessOnTaskCancellation()
guard result.exitStatus == .terminated(code: 0) else {
let swiftFormatErrorMessage: String
switch result.stderrOutput {
Expand Down
10 changes: 9 additions & 1 deletion Sources/SourceKitLSP/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public final class Workspace: Sendable {
self.semanticIndexManager = SemanticIndexManager(
index: uncheckedIndex,
buildSystemManager: buildSystemManager,
updateIndexStoreTimeout: options.indexOptions.updateIndexStoreTimeout,
testHooks: options.indexTestHooks,
indexTaskScheduler: indexTaskScheduler,
logMessageToIndexLog: logMessageToIndexLog,
Expand Down Expand Up @@ -275,17 +276,24 @@ public struct IndexOptions: Sendable {
/// Setting this to a value < 1 ensures that background indexing doesn't use all CPU resources.
public var maxCoresPercentageToUseForBackgroundIndexing: Double

/// How long to wait until we cancel an update indexstore task. This timeout should be long enough that all
/// `swift-frontend` tasks finish within it. It prevents us from blocking the index if the type checker gets stuck on
/// an expression for a long time.
public var updateIndexStoreTimeout: Duration

public init(
indexStorePath: AbsolutePath? = nil,
indexDatabasePath: AbsolutePath? = nil,
indexPrefixMappings: [PathPrefixMapping]? = nil,
listenToUnitEvents: Bool = true,
maxCoresPercentageToUseForBackgroundIndexing: Double = 1
maxCoresPercentageToUseForBackgroundIndexing: Double = 1,
updateIndexStoreTimeout: Duration = .seconds(120)
) {
self.indexStorePath = indexStorePath
self.indexDatabasePath = indexDatabasePath
self.indexPrefixMappings = indexPrefixMappings
self.listenToUnitEvents = listenToUnitEvents
self.maxCoresPercentageToUseForBackgroundIndexing = maxCoresPercentageToUseForBackgroundIndexing
self.updateIndexStoreTimeout = updateIndexStoreTimeout
}
}
29 changes: 29 additions & 0 deletions Tests/SourceKitLSPTests/BackgroundIndexingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1246,6 +1246,35 @@ final class BackgroundIndexingTests: XCTestCase {
)
_ = try await project.testClient.send(PollIndexRequest())
}

func testCancelIndexing() async throws {
try await SkipUnless.swiftPMSupportsExperimentalPrepareForIndexing()
try SkipUnless.longTestsEnabled()

var serverOptions = SourceKitLSPServer.Options.testDefault
serverOptions.experimentalFeatures.insert(.swiftpmPrepareForIndexing)
serverOptions.indexOptions.updateIndexStoreTimeout = .seconds(1)

let dateStarted = Date()
_ = try await SwiftPMTestProject(
files: [
"Test.swift": """
func slow(x: Invalid1, y: Invalid2) {
x / y / x / y / x / y / x / y
}
"""
],
serverOptions: serverOptions,
enableBackgroundIndexing: true
)
// Creating the `SwiftPMTestProject` implicitly waits for background indexing to finish.
// Preparation of `Test.swift` should finish instantly because it doesn't type check the function body.
// Type-checking the body relies on rdar://80582770, which makes the line hard to type check. We should hit the
// timeout of 1s. Adding another 2s to escalate a SIGINT (to which swift-frontend doesn't respond) to a SIGKILL mean
// that the initial indexing should be done in ~3s. 30s should be enough to always finish within this time while
// also testing that we don't wait for type checking of Test.swift to finish.
XCTAssert(Date().timeIntervalSince(dateStarted) < 30)
}
}

extension HoverResponseContents {
Expand Down