From 7fffc04e50bd90628900d9cf19ad88a92edfee7c Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 15 Jan 2025 08:25:25 -0800 Subject: [PATCH] Mark all closures in the `.stream` outputRedirection of TSC as `@Sendable` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The closures aren’t guaranteed to be called on the same thread as the process was launched, which can cause assertion failure by the concurrency runtime. rdar://142813605 --- .../SwiftPMBuildSystem.swift | 4 ++-- Sources/Diagnose/DiagnoseCommand.swift | 24 ++++++++++++------- .../UpdateIndexStoreTaskDescription.swift | 4 ++-- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index c8e92edc..b0df6b3c 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -655,8 +655,8 @@ package actor SwiftPMBuildSystem: BuiltInBuildSystem { arguments: arguments, workingDirectory: nil, outputRedirection: .stream( - stdout: { stdoutHandler.handleDataFromPipe(Data($0)) }, - stderr: { stderrHandler.handleDataFromPipe(Data($0)) } + stdout: { @Sendable bytes in stdoutHandler.handleDataFromPipe(Data(bytes)) }, + stderr: { @Sendable bytes in stderrHandler.handleDataFromPipe(Data(bytes)) } ) ) let exitStatus = result.exitStatus.exhaustivelySwitchable diff --git a/Sources/Diagnose/DiagnoseCommand.swift b/Sources/Diagnose/DiagnoseCommand.swift index 16e20785..605ee5f7 100644 --- a/Sources/Diagnose/DiagnoseCommand.swift +++ b/Sources/Diagnose/DiagnoseCommand.swift @@ -233,7 +233,8 @@ package struct DiagnoseCommand: AsyncParsableCommand { throw GenericError("Failed to create log.txt") } let fileHandle = try FileHandle(forWritingTo: outputFileUrl) - var bytesCollected = 0 + let bytesCollected = AtomicInt32(initialValue: 0) + let processExited = AtomicBool(initialValue: false) // 50 MB is an average log size collected by sourcekit-lsp diagnose. // It's a good proxy to show some progress indication for the majority of the time. let expectedLogSize = 50_000_000 @@ -247,21 +248,28 @@ package struct DiagnoseCommand: AsyncParsableCommand { "--signpost", ], outputRedirection: .stream( - stdout: { bytes in + stdout: { @Sendable bytes in try? fileHandle.write(contentsOf: bytes) - bytesCollected += bytes.count - var progress = Double(bytesCollected) / Double(expectedLogSize) + bytesCollected.value += Int32(bytes.count) + var progress = Double(bytesCollected.value) / Double(expectedLogSize) if progress > 1 { // The log is larger than we expected. Halt at 100% progress = 1 } - reportProgress(.collectingLogMessages(progress: progress), message: "Collecting log messages") + Task(priority: .high) { + // We have launched an async task to call `reportProgress`, which means that the process might have exited + // before we execute this task. To avoid overriding a more recent progress, add a guard. + if !processExited.value { + await reportProgress(.collectingLogMessages(progress: progress), message: "Collecting log messages") + } + } }, - stderr: { _ in } + stderr: { @Sendable _ in } ) ) try process.launch() try await process.waitUntilExit() + processExited.value = true #endif } @@ -343,8 +351,8 @@ package struct DiagnoseCommand: AsyncParsableCommand { let process = Process( arguments: [try swiftUrl.filePath, "--version"], outputRedirection: .stream( - stdout: { try? fileHandle.write(contentsOf: $0) }, - stderr: { _ in } + stdout: { @Sendable bytes in try? fileHandle.write(contentsOf: bytes) }, + stderr: { @Sendable _ in } ) ) try process.launch() diff --git a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift index d0cf21e7..77f1b287 100644 --- a/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift +++ b/Sources/SemanticIndex/UpdateIndexStoreTaskDescription.swift @@ -404,8 +404,8 @@ package struct UpdateIndexStoreTaskDescription: IndexTaskDescription { arguments: processArguments, workingDirectory: workingDirectory, outputRedirection: .stream( - stdout: { stdoutHandler.handleDataFromPipe(Data($0)) }, - stderr: { stderrHandler.handleDataFromPipe(Data($0)) } + stdout: { @Sendable bytes in stdoutHandler.handleDataFromPipe(Data(bytes)) }, + stderr: { @Sendable bytes in stderrHandler.handleDataFromPipe(Data(bytes)) } ) ) }