Skip to content

Ensure the DLHandle passed to SourceKitD.init(dlhandle:path:pluginPaths:initialize:) gets closed if the initializer fails #2096

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
80 changes: 42 additions & 38 deletions Sources/SourceKitD/SourceKitD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -193,52 +193,56 @@ package actor SourceKitD {
let dlopenModes: DLOpenFlags = [.lazy, .local, .first]
#endif
let dlhandle = try dlopen(path.filePath, mode: dlopenModes)
do {
try self.init(
dlhandle: dlhandle,
path: path,
pluginPaths: pluginPaths,
initialize: initialize
)
} catch {
try? dlhandle.close()
throw error
}
try self.init(
dlhandle: dlhandle,
path: path,
pluginPaths: pluginPaths,
initialize: initialize
)
}

/// Create a `SourceKitD` instance from an existing `DLHandle`. `SourceKitD` takes over ownership of the `DLHandler`
/// and will close it when the `SourceKitD` instance gets deinitialized or if the initializer throws.
package init(dlhandle: DLHandle, path: URL, pluginPaths: PluginPaths?, initialize: Bool) throws {
self.path = path
self.dylib = dlhandle
let api = try sourcekitd_api_functions_t(dlhandle)
self.api = api

// We load the plugin-related functions eagerly so the members are initialized and we don't have data races on first
// access to eg. `pluginApi`. But if one of the functions is missing, we will only emit that error when that family
// of functions is being used. For example, it is expected that the plugin functions are not available in
// SourceKit-LSP.
self.ideApiResult = Result(catching: { try sourcekitd_ide_api_functions_t(dlhandle) })
self.pluginApiResult = Result(catching: { try sourcekitd_plugin_api_functions_t(dlhandle) })
self.servicePluginApiResult = Result(catching: { try sourcekitd_service_plugin_api_functions_t(dlhandle) })

if let pluginPaths {
api.register_plugin_path?(pluginPaths.clientPlugin.path, pluginPaths.servicePlugin.path)
}
if initialize {
self.api.initialize()
}
do {
self.path = path
self.dylib = dlhandle
let api = try sourcekitd_api_functions_t(dlhandle)
self.api = api

// We load the plugin-related functions eagerly so the members are initialized and we don't have data races on first
// access to eg. `pluginApi`. But if one of the functions is missing, we will only emit that error when that family
// of functions is being used. For example, it is expected that the plugin functions are not available in
// SourceKit-LSP.
self.ideApiResult = Result(catching: { try sourcekitd_ide_api_functions_t(dlhandle) })
self.pluginApiResult = Result(catching: { try sourcekitd_plugin_api_functions_t(dlhandle) })
self.servicePluginApiResult = Result(catching: { try sourcekitd_service_plugin_api_functions_t(dlhandle) })

if initialize {
self.api.set_notification_handler { [weak self] rawResponse in
guard let self, let rawResponse else { return }
let response = SKDResponse(rawResponse, sourcekitd: self)
self.notificationHandlingQueue.async {
let handlers = await self.notificationHandlers.compactMap(\.value)
if let pluginPaths {
api.register_plugin_path?(pluginPaths.clientPlugin.path, pluginPaths.servicePlugin.path)
}
if initialize {
self.api.initialize()
}

if initialize {
self.api.set_notification_handler { [weak self] rawResponse in
guard let self, let rawResponse else { return }
let response = SKDResponse(rawResponse, sourcekitd: self)
self.notificationHandlingQueue.async {
let handlers = await self.notificationHandlers.compactMap(\.value)

for handler in handlers {
handler.notification(response)
for handler in handlers {
handler.notification(response)
}
}
}
}
} catch {
orLog("Closing dlhandle after opening sourcekitd failed") {
try? dlhandle.close()
}
throw error
}
}

Expand Down
5 changes: 4 additions & 1 deletion Sources/SourceKitD/dlopen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//

import SKLogging
import SwiftExtensions

#if os(Windows)
Expand Down Expand Up @@ -45,7 +46,9 @@ package final class DLHandle: Sendable {
}

deinit {
precondition(rawValue.value == nil, "DLHandle must be closed or explicitly leaked before destroying")
if rawValue.value != nil {
logger.fault("DLHandle must be closed or explicitly leaked before destroying")
}
}

/// The handle must not be used anymore after calling `close`.
Expand Down