Skip to content

Commit ee29e31

Browse files
authored
Merge pull request #1719 from ahoppen/explicit-bsp-shutdown
Explicitly shut down the build server when SourceKit-LSP is shut down
2 parents 9a3a0df + d25b65c commit ee29e31

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
@@ -444,6 +444,23 @@ package actor BuildSystemManager: QueueBasedMessageHandler {
444444
}
445445
}
446446

447+
/// Explicitly shut down the build server.
448+
///
449+
/// The build server is automatically shut down using a background task when `BuildSystemManager` is deallocated.
450+
/// This, however, leads to possible race conditions where the shutdown task might not finish before the test is done,
451+
/// which could result in the connection being reported as a leak. To avoid this problem, we want to explicitly shut
452+
/// down the build server when the `SourceKitLSPServer` gets shut down.
453+
package func shutdown() async {
454+
guard let buildSystemAdapter = await self.buildSystemAdapterAfterInitialized else {
455+
return
456+
}
457+
await orLog("Sending shutdown request to build server") {
458+
_ = try await buildSystemAdapter.send(BuildShutdownRequest())
459+
await buildSystemAdapter.send(OnBuildExitNotification())
460+
}
461+
self.buildSystemAdapter = nil
462+
}
463+
447464
deinit {
448465
// Shut down the build server before closing the connection to it
449466
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
@@ -1136,6 +1136,16 @@ extension SourceKitLSPServer {
11361136
await service.shutdown()
11371137
}
11381138
}
1139+
for workspace in workspaces {
1140+
taskGroup.addTask {
1141+
await orLog("Shutting down build server") {
1142+
// If the build server doesn't shut down in 1 second, don't delay SourceKit-LSP's shutdown because of it.
1143+
try await withTimeout(.seconds(2)) {
1144+
await workspace.buildSystemManager.shutdown()
1145+
}
1146+
}
1147+
}
1148+
}
11391149
}
11401150

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

0 commit comments

Comments
 (0)