From a151ca413c64aa03cdfec2bc85f8efb9fca51f59 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 27 Mar 2025 15:14:28 -0700 Subject: [PATCH] Fix a race condition leading to out-of-order notifications in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `TestSourceKitLSPClient.handle` created a new `Task`. This means that we could swap the order of notifications received from SourceKit-LSP. In most cases this doesn’t matter but `BackgroundIndexingTests.testProduceIndexLogWithTaskID` checks that the notification to start a structured log is received before the first report, which might not be the case because of the `Task` here. Change `PendingNotifications` to a class with a `ThreadSafeBox` to remove the need for a `Task`. rdar://147814254 --- Sources/SKTestSupport/TestSourceKitLSPClient.swift | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Sources/SKTestSupport/TestSourceKitLSPClient.swift b/Sources/SKTestSupport/TestSourceKitLSPClient.swift index fd3b498c..516765ff 100644 --- a/Sources/SKTestSupport/TestSourceKitLSPClient.swift +++ b/Sources/SKTestSupport/TestSourceKitLSPClient.swift @@ -52,16 +52,16 @@ fileprivate struct NotificationTimeoutError: Error, CustomStringConvertible { /// We can't use an `AsyncStream` for this because an `AsyncStream` is cancelled if a task that calls /// `AsyncStream.Iterator.next` is cancelled and we want to be able to wait for new notifications even if waiting for a /// a previous notification timed out. -actor PendingNotifications { - private var values: [any NotificationType] = [] +final class PendingNotifications: Sendable { + private let values = ThreadSafeBox<[any NotificationType]>(initialValue: []) - func add(_ value: any NotificationType) { - values.insert(value, at: 0) + nonisolated func add(_ value: any NotificationType) { + values.value.insert(value, at: 0) } func next(timeout: Duration, pollingInterval: Duration = .milliseconds(10)) async throws -> any NotificationType { for _ in 0..