From 47a940d8bdae5fe03454bf2d1ef2750f76e0e1f2 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Tue, 13 May 2025 17:24:45 +0200 Subject: [PATCH] Fix race condition that causes task cancellation to be missed in `TaskScheduler` A queued task might have been cancelled after the execution ask was started but before the task was yielded to `executionTaskCreatedContinuation`. In that case the result task will simply cancel the await on the `executionTaskCreatedStream` and hence not call `valuePropagatingCancellation` on the execution task. This means that the queued task cancellation wouldn't be propagated to the execution task. To address this, check if `resultTaskCancelled` was set and, if so, explicitly cancel the execution task here. Fixes an issue I saw in CI during PR testing. --- Sources/SemanticIndex/PreparationTaskDescription.swift | 4 +--- Sources/SemanticIndex/TaskScheduler.swift | 8 ++++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Sources/SemanticIndex/PreparationTaskDescription.swift b/Sources/SemanticIndex/PreparationTaskDescription.swift index 04626cffc..841c81bf1 100644 --- a/Sources/SemanticIndex/PreparationTaskDescription.swift +++ b/Sources/SemanticIndex/PreparationTaskDescription.swift @@ -105,9 +105,7 @@ package struct PreparationTaskDescription: IndexTaskDescription { do { try await buildSystemManager.prepare(targets: Set(targetsToPrepare)) } catch { - logger.error( - "Preparation failed: \(error.forLogging)" - ) + logger.error("Preparation failed: \(error.forLogging)") } await hooks.preparationTaskDidFinish?(self) if !Task.isCancelled { diff --git a/Sources/SemanticIndex/TaskScheduler.swift b/Sources/SemanticIndex/TaskScheduler.swift index 95060f71f..45b1de1bb 100644 --- a/Sources/SemanticIndex/TaskScheduler.swift +++ b/Sources/SemanticIndex/TaskScheduler.swift @@ -249,6 +249,14 @@ package actor QueuedTask { _isExecuting.value = true executionTask = task executionTaskCreatedContinuation.yield(task) + if self.resultTaskCancelled.value { + // The queued task might have been cancelled after the execution ask was started but before the task was yielded + // to `executionTaskCreatedContinuation`. In that case the result task will simply cancel the await on the + // `executionTaskCreatedStream` and hence not call `valuePropagatingCancellation` on the execution task. This + // means that the queued task cancellation wouldn't be propagated to the execution task. To address this, check if + // `resultTaskCancelled` was set and, if so, explicitly cancel the execution task here. + task.cancel() + } await executionStateChangedCallback?(self, .executing) return await task.value }