Skip to content

Commit d25b65c

Browse files
committed
Explicitly shut down the build server when SourceKit-LSP is shut down
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.
1 parent 8861f3a commit d25b65c

File tree

4 files changed

+31
-4
lines changed

4 files changed

+31
-4
lines changed

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,23 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
430430
}
431431
}
432432

433+
/// Explicitly shut down the build server.
434+
///
435+
/// The build server is automatically shut down using a background task when `BuildSystemManager` is deallocated.
436+
/// This, however, leads to possible race conditions where the shutdown task might not finish before the test is done,
437+
/// which could result in the connection being reported as a leak. To avoid this problem, we want to explicitly shut
438+
/// down the build server when the `SourceKitLSPServer` gets shut down.
439+
package func shutdown() async {
440+
guard let buildSystemAdapter = await self.buildSystemAdapterAfterInitialized else {
441+
return
442+
}
443+
await orLog("Sending shutdown request to build server") {
444+
_ = try await buildSystemAdapter.send(BuildShutdownRequest())
445+
await buildSystemAdapter.send(OnBuildExitNotification())
446+
}
447+
self.buildSystemAdapter = nil
448+
}
449+
433450
deinit {
434451
// Shut down the build server before closing the connection to it
435452
Task { [buildSystemAdapter, initializeResult] in

Sources/BuildSystemIntegration/ExternalBuildSystemAdapter.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ actor ExternalBuildSystemAdapter {
118118
}
119119

120120
/// Send a notification to the build server.
121-
func send(_ notification: some NotificationType) async {
121+
func send(_ notification: some NotificationType) {
122122
guard let connectionToBuildServer else {
123123
logger.error("Dropping notification because BSP server has crashed: \(notification.forLogging)")
124124
return

Sources/SKTestSupport/INPUTS/AbstractBuildServer.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def run(self):
3838

3939
try:
4040
result = self.handle_message(message)
41-
if result:
41+
if result is not None:
4242
response_message: Dict[str, object] = {
4343
"jsonrpc": "2.0",
4444
"id": message["id"],
@@ -139,8 +139,8 @@ def textdocument_sourcekitoptions(
139139
code=-32601, message=f"'textDocument/sourceKitOptions' not implemented"
140140
)
141141

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

145145
def buildtarget_sources(self, request: Dict[str, object]) -> Dict[str, object]:
146146
raise RequestError(

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,16 @@ extension SourceKitLSPServer {
12411241
await service.shutdown()
12421242
}
12431243
}
1244+
for workspace in workspaces {
1245+
taskGroup.addTask {
1246+
await orLog("Shutting down build server") {
1247+
// If the build server doesn't shut down in 1 second, don't delay SourceKit-LSP's shutdown because of it.
1248+
try await withTimeout(.seconds(2)) {
1249+
await workspace.buildSystemManager.shutdown()
1250+
}
1251+
}
1252+
}
1253+
}
12441254
}
12451255

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

0 commit comments

Comments
 (0)