Skip to content

Explicitly shut down the build server when SourceKit-LSP is shut down #1719

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
Oct 2, 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
17 changes: 17 additions & 0 deletions Sources/BuildSystemIntegration/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,23 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
}
}

/// Explicitly shut down the build server.
///
/// The build server is automatically shut down using a background task when `BuildSystemManager` is deallocated.
/// This, however, leads to possible race conditions where the shutdown task might not finish before the test is done,
/// which could result in the connection being reported as a leak. To avoid this problem, we want to explicitly shut
/// down the build server when the `SourceKitLSPServer` gets shut down.
package func shutdown() async {
guard let buildSystemAdapter = await self.buildSystemAdapterAfterInitialized else {
return
}
await orLog("Sending shutdown request to build server") {
_ = try await buildSystemAdapter.send(BuildShutdownRequest())
await buildSystemAdapter.send(OnBuildExitNotification())
}
self.buildSystemAdapter = nil
}

deinit {
// Shut down the build server before closing the connection to it
Task { [buildSystemAdapter, initializeResult] in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ actor ExternalBuildSystemAdapter {
}

/// Send a notification to the build server.
func send(_ notification: some NotificationType) async {
func send(_ notification: some NotificationType) {
guard let connectionToBuildServer else {
logger.error("Dropping notification because BSP server has crashed: \(notification.forLogging)")
return
Expand Down
6 changes: 3 additions & 3 deletions Sources/SKTestSupport/INPUTS/AbstractBuildServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def run(self):

try:
result = self.handle_message(message)
if result:
if result is not None:
response_message: Dict[str, object] = {
"jsonrpc": "2.0",
"id": message["id"],
Expand Down Expand Up @@ -139,8 +139,8 @@ def textdocument_sourcekitoptions(
code=-32601, message=f"'textDocument/sourceKitOptions' not implemented"
)

def shutdown(self, notification: Dict[str, object]) -> None:
pass
def shutdown(self, request: Dict[str, object]) -> Dict[str, object]:
return {}

def buildtarget_sources(self, request: Dict[str, object]) -> Dict[str, object]:
raise RequestError(
Expand Down
10 changes: 10 additions & 0 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,16 @@ extension SourceKitLSPServer {
await service.shutdown()
}
}
for workspace in workspaces {
taskGroup.addTask {
await orLog("Shutting down build server") {
// If the build server doesn't shut down in 1 second, don't delay SourceKit-LSP's shutdown because of it.
try await withTimeout(.seconds(2)) {
await workspace.buildSystemManager.shutdown()
}
}
}
}
}

// We have a semantic guarantee that no request or notification should be
Expand Down