mirror of
https://github.com/sudosylabs/vnidrop.git
synced 2026-08-05 02:29:55 +02:00
fix(apple): make receive-cancel actually cancel the transfer
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.
This commit is contained in:
50
apple/Tests/CoreDispatcherTests.swift
Normal file
50
apple/Tests/CoreDispatcherTests.swift
Normal file
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
41
apple/VniDrop/Core/CoreDispatcher.swift
Normal file
41
apple/VniDrop/Core/CoreDispatcher.swift
Normal file
@@ -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<T: Sendable>(_ block: @escaping @Sendable () throws -> T) async -> Result<T, Error> {
|
||||||
|
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<T: Sendable>(_ block: @escaping @Sendable () throws -> T) async -> Result<T, Error> {
|
||||||
|
await withCheckedContinuation { continuation in
|
||||||
|
interruptQueue.async { continuation.resume(returning: Result { try block() }) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -21,7 +21,7 @@ final class CoreRepository: ObservableObject, CoreGateway {
|
|||||||
// `runCore`; the underlying core is internally synchronized, so this crossing
|
// `runCore`; the underlying core is internally synchronized, so this crossing
|
||||||
// is safe. `nonisolated(unsafe)` documents that contract for Swift 6.
|
// is safe. `nonisolated(unsafe)` documents that contract for Swift 6.
|
||||||
private nonisolated(unsafe) var core: VnidropCore?
|
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
|
private lazy var sink = RepositoryEventSink { [weak self] event in
|
||||||
Task { @MainActor in self?.handle(event: event) }
|
Task { @MainActor in self?.handle(event: event) }
|
||||||
}
|
}
|
||||||
@@ -119,7 +119,9 @@ final class CoreRepository: ObservableObject, CoreGateway {
|
|||||||
// MARK: - Lifecycle actions
|
// MARK: - Lifecycle actions
|
||||||
|
|
||||||
func cancel(transferId: UInt64) async -> Result<Void, Error> {
|
func cancel(transferId: UInt64) async -> Result<Void, Error> {
|
||||||
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)
|
try self.requireCore().cancelTransfer(transferId: transferId)
|
||||||
}.map { self.refreshSnapshot() }
|
}.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.
|
/// Runs a blocking core call off the main actor and hops the result back.
|
||||||
private nonisolated func runCore<T: Sendable>(_ block: @escaping @Sendable () throws -> T) async -> Result<T, Error> {
|
private nonisolated func runCore<T: Sendable>(_ block: @escaping @Sendable () throws -> T) async -> Result<T, Error> {
|
||||||
await withCheckedContinuation { continuation in
|
await dispatcher.run(block)
|
||||||
queue.async {
|
|
||||||
let result: Result<T, Error>
|
|
||||||
do {
|
|
||||||
result = .success(try block())
|
|
||||||
} catch {
|
|
||||||
result = .failure(error)
|
|
||||||
}
|
|
||||||
continuation.resume(returning: result)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// 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<T: Sendable>(_ block: @escaping @Sendable () throws -> T) async -> Result<T, Error> {
|
||||||
|
await dispatcher.runInterrupt(block)
|
||||||
}
|
}
|
||||||
|
|
||||||
private nonisolated static func nextTransferId() -> UInt64 {
|
private nonisolated static func nextTransferId() -> UInt64 {
|
||||||
|
|||||||
Reference in New Issue
Block a user