Skip to content

Commit c36a854

Browse files
committed
Wrap request and notification ID for the purpose of logging scopes
OSLog only allows the creation of 4000 logger objects, which means that logging will crash on the 4000th request to sourcekit-lsp (which would create the 4000th os_log object because it is the 4000th distinct category in which we are logging). To circumvent this problem, only use the last two digits of the request and notification ID for the logging scope and wrap around afterwards. This should still allow us to tell apart log messages from concurrently handled requests/notifications while only using 200 os_log objects, which means we stay well clear of the 4000 limit. rdar://119221355
1 parent d931806 commit c36a854

File tree

2 files changed

+29
-26
lines changed

2 files changed

+29
-26
lines changed

Sources/LSPLogging/LoggingScope.swift

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -22,36 +22,20 @@ public final class LoggingScope {
2222
}
2323
}
2424

25-
/// Append `subscope` to the current scope name, if any exists, otherwise return
26-
/// `subscope`.
27-
private func newLoggingScopeName(_ subscope: String) -> String {
28-
if let existingScope = LoggingScope._scope {
29-
return "\(existingScope).\(subscope)"
30-
} else {
31-
return subscope
32-
}
33-
}
34-
35-
/// Create a new logging scope, which will be used as the category in any log
36-
/// messages created from the operation.
37-
///
38-
/// The name of the new scope will be any existing scope with the new scope
39-
/// appended using a `.`.
40-
///
41-
/// For example if we are currently logging scope `handleRequest` and we start a
42-
/// new scope `sourcekitd`, then the new scope has the name `handleRequest.sourcekitd`.
25+
/// Create a new logging scope, which will be used as the category in any log messages created from the operation.
4326
///
44-
/// Because `.` is used to separate scopes and sub-scopes, `subscope` should not
45-
/// contain any `.`.
27+
/// This overrides the current logging scope.
4628
///
4729
/// - Note: Since this stores the logging scope in a task-local value, it only
4830
/// works when run inside a task. Outside a task, this is a no-op.
31+
/// - Warning: Be very careful with the dynamic creation of logging scopes. The logging scope is used as the os_log
32+
/// category, os_log only supports 4000 different loggers and thus at most 4000 different scopes must be used.
4933
public func withLoggingScope<Result>(
50-
_ subscope: String,
34+
_ scope: String,
5135
_ operation: () throws -> Result
5236
) rethrows -> Result {
5337
return try LoggingScope.$_scope.withValue(
54-
newLoggingScopeName(subscope),
38+
scope,
5539
operation: operation
5640
)
5741
}
@@ -60,11 +44,11 @@ public func withLoggingScope<Result>(
6044
///
6145
/// - SeeAlso: ``withLoggingScope(_:_:)-6qtga``
6246
public func withLoggingScope<Result>(
63-
_ subscope: String,
47+
_ scope: String,
6448
_ operation: () async throws -> Result
6549
) async rethrows -> Result {
6650
return try await LoggingScope.$_scope.withValue(
67-
newLoggingScopeName(subscope),
51+
scope,
6852
operation: operation
6953
)
7054
}

Sources/SourceKitLSP/SourceKitServer.swift

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,10 @@ extension SourceKitServer: MessageHandler {
736736
messageHandlingQueue.async(metadata: TaskMetadata(params)) {
737737
signposter.emitEvent("Start handling", id: signpostID)
738738

739-
await withLoggingScope("notification-\(notificationID)") {
739+
// Only use the last two digits of the notification ID for the logging scope to avoid creating too many scopes.
740+
// See comment in `withLoggingScope`.
741+
// The last 2 digits should be sufficient to differentiate between multiple concurrently running notifications.
742+
await withLoggingScope("notification-\(notificationID % 100)") {
740743
await self.handleImpl(params, from: clientID)
741744
signposter.endInterval("Notification", state, "Done")
742745
}
@@ -783,7 +786,10 @@ extension SourceKitServer: MessageHandler {
783786

784787
let task = messageHandlingQueue.async(metadata: TaskMetadata(params)) {
785788
signposter.emitEvent("Start handling", id: signpostID)
786-
await withLoggingScope("request-\(id)") {
789+
// Only use the last two digits of the request ID for the logging scope to avoid creating too many scopes.
790+
// See comment in `withLoggingScope`.
791+
// The last 2 digits should be sufficient to differentiate between multiple concurrently running requests.
792+
await withLoggingScope("request-\(id.numericValue % 100)") {
787793
await self.handleImpl(params, id: id, from: clientID, reply: reply)
788794
signposter.endInterval("Request", state, "Done")
789795
}
@@ -2331,3 +2337,16 @@ extension WorkspaceSymbolItem {
23312337
)
23322338
}
23332339
}
2340+
2341+
fileprivate extension RequestID {
2342+
/// Returns a numeric value for this request ID.
2343+
///
2344+
/// For request IDs that are numbers, this is straightforward. For string-based request IDs, this uses a hash to
2345+
/// convert the string into a number.
2346+
var numericValue: Int {
2347+
switch self {
2348+
case .number(let number): return number
2349+
case .string(let string): return Int(string) ?? string.hashValue
2350+
}
2351+
}
2352+
}

0 commit comments

Comments
 (0)