From 081b59815c232c4a869c8c09c181f89735fab7e8 Mon Sep 17 00:00:00 2001 From: cdricms <36056008+cdricms@users.noreply.github.com> Date: Thu, 23 Jul 2026 18:30:55 +0200 Subject: [PATCH] fix(apple): make receive-cancel actually cancel the transfer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Cancel button on an in-progress receive did nothing. CoreRepository funnelled every core call through one serial DispatchQueue, but `receive` is a blocking core call that occupies that queue for the whole transfer. The tapped `cancelTransfer` was enqueued behind the in-flight `receive` on the same serial queue, so it could never run until `receive` returned — which it never would, because it was waiting to be cancelled. A deadlock the button couldn't escape. The Rust core is explicitly designed for cancel to arrive from another thread mid-receive (VnidropCore.block_on uses a shared runtime handle for exactly this). Extracts the two-lane dispatch into a CoreDispatcher: a serial lane for ordered calls and a separate concurrent lane for interrupt-style calls, and routes cancel through the latter so the signal reaches the core and unblocks the receive. Adds CoreDispatcherTests, including a regression guard that an interrupt completes while the serial lane is blocked. --- apple/Tests/CoreDispatcherTests.swift | 50 +++++++++++++++++++++++++ apple/VniDrop/Core/CoreDispatcher.swift | 41 ++++++++++++++++++++ apple/VniDrop/Core/CoreRepository.swift | 25 ++++++------- 3 files changed, 103 insertions(+), 13 deletions(-) create mode 100644 apple/Tests/CoreDispatcherTests.swift create mode 100644 apple/VniDrop/Core/CoreDispatcher.swift diff --git a/apple/Tests/CoreDispatcherTests.swift b/apple/Tests/CoreDispatcherTests.swift new file mode 100644 index 0000000..4a71272 --- /dev/null +++ b/apple/Tests/CoreDispatcherTests.swift @@ -0,0 +1,50 @@ +import XCTest +@testable import VniDrop + +final class CoreDispatcherTests: XCTestCase { + + /// Regression guard for the receive-cancel deadlock: an interrupt-lane call + /// must complete even while the serial lane is occupied by a blocking call. + /// With a single shared queue (the old design) the interrupt would be stuck + /// behind the blocked `receive`, and this would time out. + func testInterruptCompletesWhileSerialLaneIsBlocked() async { + let dispatcher = CoreDispatcher() + let serialEntered = DispatchSemaphore(value: 0) + let releaseSerial = DispatchSemaphore(value: 0) + + // Occupy the serial lane with a call that blocks until we release it. + let serialTask = Task { + await dispatcher.run { + serialEntered.signal() + releaseSerial.wait() + } + } + XCTAssertEqual(serialEntered.wait(timeout: .now() + 2), .success, "serial lane never started") + + // The interrupt lane must run despite the serial lane being blocked. + let interruptDone = DispatchSemaphore(value: 0) + Task.detached { + _ = await dispatcher.runInterrupt { 42 } + interruptDone.signal() + } + XCTAssertEqual( + interruptDone.wait(timeout: .now() + 2), .success, + "interrupt lane was blocked behind the occupied serial lane") + + releaseSerial.signal() + _ = await serialTask.value + } + + func testRunPropagatesValuesAndErrors() async { + let dispatcher = CoreDispatcher() + + let value = await dispatcher.run { 7 } + XCTAssertEqual(try? value.get(), 7) + + let failure = await dispatcher.run { () -> Int in throw TestError.unimplemented } + switch failure { + case .success: XCTFail("expected the thrown error to propagate") + case .failure(let error): XCTAssertTrue(error is TestError) + } + } +} diff --git a/apple/VniDrop/Core/CoreDispatcher.swift b/apple/VniDrop/Core/CoreDispatcher.swift new file mode 100644 index 0000000..efffaea --- /dev/null +++ b/apple/VniDrop/Core/CoreDispatcher.swift @@ -0,0 +1,41 @@ +import Foundation + +/// Dispatch-queue labels for the core's serial and interrupt lanes. +enum QueueLabel { + static let core = "com.vnidrop.core" + static let interrupt = "com.vnidrop.core.interrupt" +} + +/// Two-lane dispatcher for blocking core calls. +/// +/// `run` serializes calls on one queue so the core is driven from a single lane. +/// `runInterrupt` uses a *separate* concurrent lane, so an interrupt-style call +/// (cancel) can reach the core while a blocking call (`receive`) still occupies +/// the serial lane. The core is internally synchronized and explicitly supports +/// cancel arriving from another thread mid-receive (see VnidropCore.block_on); +/// a single shared queue would deadlock it. +final class CoreDispatcher: Sendable { + private let serialQueue: DispatchQueue + private let interruptQueue: DispatchQueue + + init(label: String = QueueLabel.core, interruptLabel: String = QueueLabel.interrupt) { + serialQueue = DispatchQueue(label: label, qos: .userInitiated) + interruptQueue = DispatchQueue(label: interruptLabel, qos: .userInitiated, attributes: .concurrent) + } + + /// Runs a blocking core call on the serial lane and hops the result back. + func run(_ block: @escaping @Sendable () throws -> T) async -> Result { + await withCheckedContinuation { continuation in + serialQueue.async { continuation.resume(returning: Result { try block() }) } + } + } + + /// Like `run`, but off the serial lane so it can interrupt a blocking call in + /// flight there (e.g. cancel a `receive`). Only use for core calls that are + /// safe to run concurrently with another core call. + func runInterrupt(_ block: @escaping @Sendable () throws -> T) async -> Result { + await withCheckedContinuation { continuation in + interruptQueue.async { continuation.resume(returning: Result { try block() }) } + } + } +} diff --git a/apple/VniDrop/Core/CoreRepository.swift b/apple/VniDrop/Core/CoreRepository.swift index f455a81..f7ea1a3 100644 --- a/apple/VniDrop/Core/CoreRepository.swift +++ b/apple/VniDrop/Core/CoreRepository.swift @@ -21,7 +21,7 @@ final class CoreRepository: ObservableObject, CoreGateway { // `runCore`; the underlying core is internally synchronized, so this crossing // is safe. `nonisolated(unsafe)` documents that contract for Swift 6. private nonisolated(unsafe) var core: VnidropCore? - private let queue = DispatchQueue(label: "com.vnidrop.core", qos: .userInitiated) + private let dispatcher = CoreDispatcher() private lazy var sink = RepositoryEventSink { [weak self] event in Task { @MainActor in self?.handle(event: event) } } @@ -119,7 +119,9 @@ final class CoreRepository: ObservableObject, CoreGateway { // MARK: - Lifecycle actions func cancel(transferId: UInt64) async -> Result { - await runCore { + // Off the serial `queue`: a receive in flight is blocking it, and the + // cancel signal must reach the core to unblock that receive. + await runInterrupt { try self.requireCore().cancelTransfer(transferId: transferId) }.map { self.refreshSnapshot() } } @@ -253,17 +255,14 @@ final class CoreRepository: ObservableObject, CoreGateway { /// Runs a blocking core call off the main actor and hops the result back. private nonisolated func runCore(_ block: @escaping @Sendable () throws -> T) async -> Result { - await withCheckedContinuation { continuation in - queue.async { - let result: Result - do { - result = .success(try block()) - } catch { - result = .failure(error) - } - continuation.resume(returning: result) - } - } + await dispatcher.run(block) + } + + /// Like `runCore`, but off the serial lane so it can interrupt a blocking + /// call in flight there (e.g. cancel a `receive`). Only use for core calls + /// that are safe to run concurrently with another core call. + private nonisolated func runInterrupt(_ block: @escaping @Sendable () throws -> T) async -> Result { + await dispatcher.runInterrupt(block) } private nonisolated static func nextTransferId() -> UInt64 {