From a510ca61da9138e0c3d969b6004a887e350b17a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 15:09:52 +0200 Subject: [PATCH 1/6] =?UTF-8?q?SPIKE:=20node.onTeardown=20=E2=80=94=20asyn?= =?UTF-8?q?c=20work=20that=20outlives=20its=20model,=20hosted=20by=20the?= =?UTF-8?q?=20test=20harness?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 (1M context) --- .../SwiftModel/Internal/Cancellables.swift | 8 +- .../SwiftModel/Internal/Cancellations.swift | 12 +- Sources/SwiftModel/Internal/ModelAccess.swift | 6 + Sources/SwiftModel/Internal/TestAccess.swift | 23 +++- .../Internal/TestExecutorDrive.swift | 2 +- Sources/SwiftModel/ModelNode+Teardown.swift | 66 ++++++++++ .../Testing/ModelTestingSupport.swift | 5 + Tests/SwiftModelTests/TeardownWorkTests.swift | 115 ++++++++++++++++++ 8 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 Sources/SwiftModel/ModelNode+Teardown.swift create mode 100644 Tests/SwiftModelTests/TeardownWorkTests.swift diff --git a/Sources/SwiftModel/Internal/Cancellables.swift b/Sources/SwiftModel/Internal/Cancellables.swift index 38226bec..989dab16 100644 --- a/Sources/SwiftModel/Internal/Cancellables.swift +++ b/Sources/SwiftModel/Internal/Cancellables.swift @@ -72,7 +72,12 @@ final class TaskCancellable: Cancellable, InternalCancellable, @unchecked Sendab let _hasStartedRunningBox: LockIsolated var hasStartedRunning: Bool { _hasStartedRunningBox.value } - init(modelName: String, taskName: String, fileAndLine: FileAndLine, context: AnyContext, hasStartedRunningBox: LockIsolated, task: @escaping @Sendable (@escaping @Sendable () -> Void) -> Task) { + convenience init(modelName: String, taskName: String, fileAndLine: FileAndLine, context: AnyContext, hasStartedRunningBox: LockIsolated, task: @escaping @Sendable (@escaping @Sendable () -> Void) -> Task) { + // See the AB-BA note in the designated init: resolve the registry before any lock. + self.init(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: context.cancellations, hasStartedRunningBox: hasStartedRunningBox, task: task) + } + + init(modelName: String, taskName: String, fileAndLine: FileAndLine, cancellations: Cancellations, hasStartedRunningBox: LockIsolated, task: @escaping @Sendable (@escaping @Sendable () -> Void) -> Task) { // Assigned before `cancellations.register(self)` below publishes this // instance to any settle thread — see `_hasStartedRunningBox`. self._hasStartedRunningBox = hasStartedRunningBox @@ -98,7 +103,6 @@ final class TaskCancellable: Cancellable, InternalCancellable, @unchecked Sendab // whenever a leaf lock is held, do not evaluate anything that reaches a // context lock — including capture-list expressions, which are evaluated at // closure-formation time, i.e. inside the enclosing critical section. - let cancellations = context.cancellations self.cancellations = cancellations let id = cancellations.nextId self.id = id diff --git a/Sources/SwiftModel/Internal/Cancellations.swift b/Sources/SwiftModel/Internal/Cancellations.swift index 6191f3ac..1637e425 100644 --- a/Sources/SwiftModel/Internal/Cancellations.swift +++ b/Sources/SwiftModel/Internal/Cancellations.swift @@ -94,10 +94,20 @@ final class Cancellations: @unchecked Sendable { } } + /// IDs of everything currently registered (SPIKE: used to tell mid-test teardown + /// work from work the harness's own end-of-test teardown started). + var registeredIDs: Set { + lock { Set(registered.keys) } + } + var activeTasks: [(modelName: String, tasks: [(name: String, fileAndLine: FileAndLine)])] { + activeTasks(only: nil) + } + + func activeTasks(only ids: Set?) -> [(modelName: String, tasks: [(name: String, fileAndLine: FileAndLine)])] { lock { // Sort by task ID (registration order) for stable diagnostic output. - registered.values.reduce(into: [String: [(id: Int, name: String, fileAndLine: FileAndLine)]]()) { dict, c in + registered.filter { ids?.contains($0.key) ?? true }.values.reduce(into: [String: [(id: Int, name: String, fileAndLine: FileAndLine)]]()) { dict, c in if let task = c as? TaskCancellable { dict[task.modelName, default: []].append((task.id, task.taskName, task.fileAndLine)) } diff --git a/Sources/SwiftModel/Internal/ModelAccess.swift b/Sources/SwiftModel/Internal/ModelAccess.swift index 2874739a..717a42bd 100644 --- a/Sources/SwiftModel/Internal/ModelAccess.swift +++ b/Sources/SwiftModel/Internal/ModelAccess.swift @@ -139,6 +139,12 @@ class ModelAccess: ModelAccessReference, @unchecked Sendable { /// Default: no-op. `TestAccess` overrides to fire `_noteActivity`. func taskBodyStarted() {} + /// SPIKE (async teardown work): the store that hosts `node.onTeardown` work once + /// its model has been removed. `nil` in production — the work runs as a plain, + /// untracked task. `TestAccess` returns a store it owns, so the work stays visible + /// to `settle()` and the end-of-test task check after the model is gone. + var teardownWorkStore: Cancellations? { nil } + /// Records that a reactive body (`node.forEach` / `node.onChange`) delivered /// an element, keyed by its source location. Powers `settle()`'s runaway /// diagnostic: a registration that keeps firing right up to a settle timeout diff --git a/Sources/SwiftModel/Internal/TestAccess.swift b/Sources/SwiftModel/Internal/TestAccess.swift index 22027271..5de68ea7 100644 --- a/Sources/SwiftModel/Internal/TestAccess.swift +++ b/Sources/SwiftModel/Internal/TestAccess.swift @@ -517,6 +517,23 @@ final class TestAccess: ModelAccess, @unchecked Sendable { _noteActivity() } + /// SPIKE: hosts `node.onTeardown` work after its model is removed — see + /// `ModelAccess.teardownWorkStore`. Never sealed with the model tree; cancelled + /// after the final exhaustion check (and by `Cancellations.deinit`). + let teardownWork = Cancellations() + override var teardownWorkStore: Cancellations? { teardownWork } + + /// Teardown work eligible for the end-of-test "still running" report: only work the + /// TEST started (by removing a model mid-test). Work started by the harness's own + /// end-of-test teardown is cancelled afterwards, not reported — the same way + /// `onActivate` tasks are cancelled rather than reported. `nil` = report all. + var reportableTeardownWork: Set? + + /// Pending-start across the model tree AND hosted teardown work. + var hasPendingStartWork: Bool { + context.hasPendingStartTask || teardownWork.hasPendingStartTask + } + // MARK: - Runaway diagnostic (settle-timeout) /// Per-call-site reactive-body fire counts, keyed by source location. A @@ -1487,7 +1504,7 @@ final class TestAccess: ModelAccess, @unchecked Sendable { // window so we keep polling rather than hang forever on a // task that never schedules (the total budget catches that // case as a normal settle timeout). - if !pastBudget && context.hasPendingStartTask { + if !pastBudget && hasPendingStartWork { let newDeadline = Self._quietDeadline(nowNs: now, quietWindowNs: quietWindowNs, budgetEndNs: pending.totalBudgetEndNs) pending.deadlineNs = newDeadline let entryId = pending.id @@ -1551,7 +1568,7 @@ final class TestAccess: ModelAccess, @unchecked Sendable { // `.settled` branch): even with bg idle, if a registered // `TaskCancellable` body hasn't executed once yet, we must // keep waiting. Re-arm GTS and abandon this fire. - if case .settled(let quietWindowNs, _) = pending.mode, context.hasPendingStartTask { + if case .settled(let quietWindowNs, _) = pending.mode, hasPendingStartWork { let now = monotonicNanoseconds() let newDeadline = Self._quietDeadline(nowNs: now, quietWindowNs: quietWindowNs, budgetEndNs: pending.totalBudgetEndNs) pending.deadlineNs = newDeadline @@ -1629,7 +1646,7 @@ final class TestAccess: ModelAccess, @unchecked Sendable { func checkExhaustion(at fileAndLine: FileAndLine, includeUpdates: Bool, checkTasks: Bool = false, capturedUpdates: [PartialKeyPath: [ValueUpdate]]? = nil) { if checkTasks { - for info in context.activeTasks { + for info in context.activeTasks + teardownWork.activeTasks(only: lock { reportableTeardownWork }) { let taskWord = info.tasks.count == 1 ? "task" : "tasks" fail("Models of type `\(info.modelName)` have \(info.tasks.count) active \(taskWord) still running", for: .tasks, at: fileAndLine) diff --git a/Sources/SwiftModel/Internal/TestExecutorDrive.swift b/Sources/SwiftModel/Internal/TestExecutorDrive.swift index f5766599..ea737119 100644 --- a/Sources/SwiftModel/Internal/TestExecutorDrive.swift +++ b/Sources/SwiftModel/Internal/TestExecutorDrive.swift @@ -352,7 +352,7 @@ extension TestAccess { await exec.waitUntilIdleOrDeadline(checkDeadline) if !bg.isIdle { await bg.waitForCurrentItems(deadline: checkDeadline) } if !main.isIdle { await main.waitForCurrentItems(deadline: checkDeadline) } - let idleNow = exec.isExecutorIdle && bg.isIdle && main.isIdle && !self.context.hasPendingStartTask + let idleNow = exec.isExecutorIdle && bg.isIdle && main.isIdle && !self.hasPendingStartWork if idleNow { // Debounce against COMPLETIONS too, not just writes and // enqueues (`exec.activityNs` when idle = max(birth, diff --git a/Sources/SwiftModel/ModelNode+Teardown.swift b/Sources/SwiftModel/ModelNode+Teardown.swift new file mode 100644 index 00000000..ea9f8545 --- /dev/null +++ b/Sources/SwiftModel/ModelNode+Teardown.swift @@ -0,0 +1,66 @@ +import Foundation +import Dependencies +import ConcurrencyExtras + +public extension ModelNode { + /// SPIKE — async work that starts when this model is deactivated and is allowed to + /// outlive it (an audio fade-out, a final analytics flush). + /// + /// Unlike starting a `Task` from `onCancel`, the work is registered while the model + /// is still live, so it cannot be dropped by a teardown that has already sealed the + /// tree. It captures this model's dependencies at registration and runs with them. + /// + /// The model is gone while `operation` runs: capture the values you need up front, + /// and don't touch `node`. The work must tolerate cancellation. + /// + /// - Production: a plain task, owned by no model. + /// - `.modelTesting`: hosted by the test — it runs on the test's executor, `settle()` + /// waits for it, and work still running at the end of the test is reported as an + /// active task. + @discardableResult + func onTeardown(_ name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, operation: @escaping @Sendable () async -> Void) -> Cancellable { + guard let context = enforcedContext() else { return EmptyCancellable() } + + let fileAndLine = FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column) + let taskName = name ?? "\(function) @ \(fileAndLine.description)" + let modelName = typeDescription + // Everything the work needs is resolved NOW, while the model is live: after + // removal the context is sealed and its root may be gone. + let dependencies = context.capturedDependencies + let executor = _TestExecutorBox.current + let access = context.rootParent.modelAccess + let store = access?.teardownWorkStore + + return AnyCancellable(cancellations: context.cancellations) { + _startTeardownWork(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, store: store, access: access, dependencies: dependencies, executor: executor, priority: priority, operation: operation) + } + } +} + +private func _startTeardownWork(modelName: String, taskName: String, fileAndLine: FileAndLine, store: Cancellations?, access: ModelAccess?, dependencies: DependencyValues, executor: (any Sendable)?, priority: TaskPriority?, operation: @escaping @Sendable () async -> Void) { + let hasStartedRunningBox = LockIsolated(false) + let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in + let body = { @Sendable () async throws -> Void in + await DependencyValues.$_current.withValue(dependencies) { + defer { onDone() } + hasStartedRunningBox.setValue(true) + access?.taskBodyStarted() + await operation() + } + } + #if canImport(Dispatch) + if #available(macOS 15.0, iOS 18.0, tvOS 18.0, watchOS 11.0, *), + let exec = executor as? _DrainTestExecutor { + return Task(executorPreference: exec, priority: priority, operation: body) + } + #endif + return Task(name: taskName, priority: priority, operation: body) + } + + guard let store else { + // Production: nobody to report to — just run it. + _ = makeTask {} + return + } + _ = TaskCancellable(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: store, hasStartedRunningBox: hasStartedRunningBox, task: makeTask) +} diff --git a/Sources/SwiftModel/Testing/ModelTestingSupport.swift b/Sources/SwiftModel/Testing/ModelTestingSupport.swift index c633ae4b..d0d041af 100644 --- a/Sources/SwiftModel/Testing/ModelTestingSupport.swift +++ b/Sources/SwiftModel/Testing/ModelTestingSupport.swift @@ -323,6 +323,10 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch // The seal makes that drain unnecessary. tester.access.context.sealRecursively() + // SPIKE: teardown work already running now was started by the test itself. + let midTestTeardownWork = tester.access.teardownWork.registeredIDs + tester.access.lock { tester.access.reportableTeardownWork = midTestTeardownWork } + // Phase 2: Cancel all currently-registered onActivate tasks. tester.access.context.cancelAllRecursively(for: ContextCancellationKey.onActivate) @@ -334,6 +338,7 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch tester.access.checkExhaustion(at: fileAndLine, includeUpdates: false, checkTasks: true) tester.access.context.onRemoval() + tester.access.teardownWork.cancelAll() } package func cancelAndCleanup() { diff --git a/Tests/SwiftModelTests/TeardownWorkTests.swift b/Tests/SwiftModelTests/TeardownWorkTests.swift new file mode 100644 index 00000000..e930f099 --- /dev/null +++ b/Tests/SwiftModelTests/TeardownWorkTests.swift @@ -0,0 +1,115 @@ +import Testing +@testable import SwiftModel +import ConcurrencyExtras +import Clocks +import Foundation +import IssueReporting + +// SPIKE acceptance tests for `node.onTeardown` — async work that starts when a model +// is deactivated and outlives it. The motivating case is an audio fade that must keep +// running after its player's model is removed (a stream switch), stepping on an +// injected clock. Started as a raw `Task` from `onCancel`, such work is invisible to +// `settle()`/`expect` and starves under parallel test load; started with `node.task` +// from `onCancel`, it is dropped because the model's store is already sealed. + +@Model private struct FadingPlayer { + let events: TestProbe + + func onActivate() { + // Captured while live: the model is gone when the fade runs. + let clock = node.continuousClock + let events = events + node.onTeardown("fade") { + for step in 1...10 { + do { + try await clock.sleep(for: .milliseconds(30)) + } catch { + events("cancelled at \(step)") + return + } + } + events("stopped") + } + } +} + +@Model private struct PlayerHost { + var player: FadingPlayer? +} + +@Suite(.modelTesting) +struct TeardownWorkTests { + // A stream switch: only the player is removed. The fade parks on a frozen clock + // and steps forward only as the test advances it — deterministic, no polling. + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func fadeParksOnFrozenClockAndStepsWithIt() async { + let clock = TestClock() + let events = TestProbe() + let host = PlayerHost(player: FadingPlayer(events: events)).withAnchor { + $0.continuousClock = clock + } + + host.player = nil + for _ in 1...9 { + await settle() // fade parked on its next sleep + await clock.advance(by: .milliseconds(30)) + } + await settle() + #expect(events.count == 0) // one step left: still fading + + await clock.advance(by: .milliseconds(30)) + await expect(events.wasCalled(with: "stopped")) + } + + // End of test with the fade still parked: the harness reports it as a running task + // instead of silently leaking it. + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func fadeStillRunningAtEndOfTestIsReported() async { + let reporter = CapturingIssueReporter() + await withIssueReporters([reporter]) { + await withModelTesting { + let host = PlayerHost(player: FadingPlayer(events: TestProbe())).withAnchor { + $0.continuousClock = TestClock() + } + host.player = nil + await settle() + } + } + #expect(reporter.messages.contains { $0.contains("Active task 'fade' of `FadingPlayer` still running") }) + } + + // Removed by the harness's own end-of-test teardown: the fade is started (not + // dropped), but — like `onActivate` tasks — cancelled after cleanup rather than + // reported, even when it parks on a clock nobody advances. + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func fadeStartedByEndOfTestTeardownIsCancelledNotReported() async { + let reporter = CapturingIssueReporter() + let events = TestProbe() + await withIssueReporters([reporter]) { + await withModelTesting { + _ = PlayerHost(player: FadingPlayer(events: events)).withAnchor { + $0.continuousClock = TestClock() + } + } + } + // Cancellation reaches the parked sleep asynchronously. + try? await waitUntil(events.count == 1) + #expect(events.values.map { "\($0)" } == ["cancelled at 1"]) + #expect(reporter.messages.isEmpty, "\(reporter.messages)") + } +} + +// Production path (no harness): the whole tree is released, and the fade — which a +// `node.task` started from `onCancel` would lose — still runs to completion. +struct TeardownWorkReleaseTests { + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func fadeRunsWhenWholeTreeIsReleased() async throws { + let events = TestProbe() + await waitUntilRemoved { + PlayerHost(player: FadingPlayer(events: events)).withAnchor { + $0.continuousClock = ImmediateClock() + } + } + try await waitUntil(events.values.map { "\($0)" } == ["stopped"]) + } +} From 96f57b37c777f22cc004e39163ee4453ea244144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 15:59:37 +0200 Subject: [PATCH 2/6] =?UTF-8?q?SPIKE:=20onSignal/signal=20=E2=80=94=20awai?= =?UTF-8?q?table=20keyed=20signals=20with=20a=20final=20call=20on=20remova?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onSignal(key, once:, cancelPrevious:) registers an async handler; signal(key, to:) reaches handlers like send(_:to:) (default: self + descendants), runs them concurrently and returns when they're done. Every handler also gets one final call with cause .removed when its model is removed, hosted outside the model (plain task in production, the test harness's store in tests). onTeardown is a removal-only handler. Cancelling a registration unregisters it. The harness's own end-of-test teardown does not start removal calls; removals the test triggers are tracked by settle() and reported if still running. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../SwiftModel/Internal/Cancellations.swift | 23 +- Sources/SwiftModel/Internal/ModelAccess.swift | 4 + Sources/SwiftModel/Internal/TestAccess.swift | 13 +- Sources/SwiftModel/ModelNode+Signal.swift | 220 ++++++++++++++++++ Sources/SwiftModel/ModelNode+Teardown.swift | 66 ------ .../Testing/ModelTestingSupport.swift | 6 +- Tests/SwiftModelTests/SignalTests.swift | 213 +++++++++++++++++ Tests/SwiftModelTests/TeardownWorkTests.swift | 15 +- 8 files changed, 466 insertions(+), 94 deletions(-) create mode 100644 Sources/SwiftModel/ModelNode+Signal.swift delete mode 100644 Sources/SwiftModel/ModelNode+Teardown.swift create mode 100644 Tests/SwiftModelTests/SignalTests.swift diff --git a/Sources/SwiftModel/Internal/Cancellations.swift b/Sources/SwiftModel/Internal/Cancellations.swift index 1637e425..4b5d45d9 100644 --- a/Sources/SwiftModel/Internal/Cancellations.swift +++ b/Sources/SwiftModel/Internal/Cancellations.swift @@ -30,6 +30,17 @@ final class Cancellations: @unchecked Sendable { lock { _sealed = true } } + /// Sealed stores are being torn down (model removal / end-of-test teardown); an + /// `onCancel()` arriving from one is a removal, not a user cancellation. + var isSealed: Bool { + lock { _sealed } + } + + /// Registered cancellables of a given type (SPIKE: signal handler lookup). + func registered(of type: T.Type) -> [T] { + lock { registered.values.compactMap { $0 as? T } } + } + func register(_ c: InternalCancellable) { let shouldImmediatelyCancel: Bool = lock { if _sealed { return true } @@ -94,20 +105,10 @@ final class Cancellations: @unchecked Sendable { } } - /// IDs of everything currently registered (SPIKE: used to tell mid-test teardown - /// work from work the harness's own end-of-test teardown started). - var registeredIDs: Set { - lock { Set(registered.keys) } - } - var activeTasks: [(modelName: String, tasks: [(name: String, fileAndLine: FileAndLine)])] { - activeTasks(only: nil) - } - - func activeTasks(only ids: Set?) -> [(modelName: String, tasks: [(name: String, fileAndLine: FileAndLine)])] { lock { // Sort by task ID (registration order) for stable diagnostic output. - registered.filter { ids?.contains($0.key) ?? true }.values.reduce(into: [String: [(id: Int, name: String, fileAndLine: FileAndLine)]]()) { dict, c in + registered.values.reduce(into: [String: [(id: Int, name: String, fileAndLine: FileAndLine)]]()) { dict, c in if let task = c as? TaskCancellable { dict[task.modelName, default: []].append((task.id, task.taskName, task.fileAndLine)) } diff --git a/Sources/SwiftModel/Internal/ModelAccess.swift b/Sources/SwiftModel/Internal/ModelAccess.swift index 717a42bd..829d4643 100644 --- a/Sources/SwiftModel/Internal/ModelAccess.swift +++ b/Sources/SwiftModel/Internal/ModelAccess.swift @@ -145,6 +145,10 @@ class ModelAccess: ModelAccessReference, @unchecked Sendable { /// to `settle()` and the end-of-test task check after the model is gone. var teardownWorkStore: Cancellations? { nil } + /// SPIKE: `true` while the test harness tears down the model tree at the end of a + /// test — removal calls are skipped then. Always `false` in production. + var isInHarnessTeardown: Bool { false } + /// Records that a reactive body (`node.forEach` / `node.onChange`) delivered /// an element, keyed by its source location. Powers `settle()`'s runaway /// diagnostic: a registration that keeps firing right up to a settle timeout diff --git a/Sources/SwiftModel/Internal/TestAccess.swift b/Sources/SwiftModel/Internal/TestAccess.swift index 5de68ea7..d71cc9ac 100644 --- a/Sources/SwiftModel/Internal/TestAccess.swift +++ b/Sources/SwiftModel/Internal/TestAccess.swift @@ -523,11 +523,12 @@ final class TestAccess: ModelAccess, @unchecked Sendable { let teardownWork = Cancellations() override var teardownWorkStore: Cancellations? { teardownWork } - /// Teardown work eligible for the end-of-test "still running" report: only work the - /// TEST started (by removing a model mid-test). Work started by the harness's own - /// end-of-test teardown is cancelled afterwards, not reported — the same way - /// `onActivate` tasks are cancelled rather than reported. `nil` = report all. - var reportableTeardownWork: Set? + /// Set once the harness starts its own end-of-test teardown. Removal calls + /// (`onSignal` final call, `onTeardown`) are NOT started from then on: the test + /// didn't cause that removal, so its work is not the test's concern — the same way + /// `onActivate` tasks are cancelled rather than reported. + let isHarnessTeardown = LockIsolated(false) + override var isInHarnessTeardown: Bool { isHarnessTeardown.value } /// Pending-start across the model tree AND hosted teardown work. var hasPendingStartWork: Bool { @@ -1646,7 +1647,7 @@ final class TestAccess: ModelAccess, @unchecked Sendable { func checkExhaustion(at fileAndLine: FileAndLine, includeUpdates: Bool, checkTasks: Bool = false, capturedUpdates: [PartialKeyPath: [ValueUpdate]]? = nil) { if checkTasks { - for info in context.activeTasks + teardownWork.activeTasks(only: lock { reportableTeardownWork }) { + for info in context.activeTasks + teardownWork.activeTasks { let taskWord = info.tasks.count == 1 ? "task" : "tasks" fail("Models of type `\(info.modelName)` have \(info.tasks.count) active \(taskWord) still running", for: .tasks, at: fileAndLine) diff --git a/Sources/SwiftModel/ModelNode+Signal.swift b/Sources/SwiftModel/ModelNode+Signal.swift new file mode 100644 index 00000000..caa95d40 --- /dev/null +++ b/Sources/SwiftModel/ModelNode+Signal.swift @@ -0,0 +1,220 @@ +import Foundation +import Dependencies +import ConcurrencyExtras + +// SPIKE — signals: an awaitable, repeatable request that reaches related models (like +// `send`), plus a guaranteed final call when a handler's model is removed. The removal +// call runs AFTER the model is gone, so it must not be hosted by the model: in +// production it is a plain task, in `.modelTesting` it is hosted by the test harness +// (runs on the test's executor, seen by `settle()`, reported if still running at the +// end of the test when the test itself triggered it). + +/// Why a signal handler is running. +public enum SignalCause: Sendable, Equatable { + /// `signal(_:to:)` reached the handler. The model is live: `node` may be used. + case requested + /// The handler's model was removed. The model is gone: use only captured values. + case removed +} + +public extension ModelNode { + /// Registers an async handler for `signal(key, to:)` requests. + /// + /// The handler also gets exactly one final call with `.removed` when this model is + /// removed (unless `once` and it already ran). Runs of one handler are serialized + /// (a new request waits for the running one), or with `cancelPrevious` the new one + /// cancels it. Different handlers always run concurrently. + /// + /// Cancelling the returned `Cancellable` (or `cancelAll(for:)` on a key it was + /// registered under) unregisters the handler: it won't run again, not even on removal. + @discardableResult + func onSignal(_ key: some Hashable & Sendable, once: Bool = false, cancelPrevious: Bool = false, name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { + _registerSignalHandler(match: .key(CancellableKey(key: key)), once: once, cancelPrevious: cancelPrevious, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column), perform: perform) + } + + /// Registers an async handler for every `signal` request (any key), plus the final + /// `.removed` call. See `onSignal(_:once:cancelPrevious:…)`. + @discardableResult + func onSignal(once: Bool = false, cancelPrevious: Bool = false, name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { + _registerSignalHandler(match: .any, once: once, cancelPrevious: cancelPrevious, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column), perform: perform) + } + + /// Async work that starts when this model is removed and may outlive it — a + /// removal-only handler (never reached by `signal`). Capture what you need up front. + @discardableResult + func onTeardown(_ name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, operation: @escaping @Sendable () async -> Void) -> Cancellable { + _registerSignalHandler(match: .removalOnly, once: true, cancelPrevious: false, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column)) { _ in + await operation() + } + } + + /// Runs every handler registered for `key` on the models `relation` reaches, + /// concurrently, and returns when all of those runs have finished. + func signal(_ key: some Hashable & Sendable, to relation: ModelRelation = [.self, .descendants]) async { + await _signal(CancellableKey(key: key), to: relation) + } + + /// Runs every signal handler (any key) on the models `relation` reaches. + func signal(to relation: ModelRelation = [.self, .descendants]) async { + await _signal(nil, to: relation) + } +} + +private extension ModelNode { + func _registerSignalHandler(match: SignalHandler.Match, once: Bool, cancelPrevious: Bool, name: String?, function: StaticString, priority: TaskPriority?, fileAndLine: FileAndLine, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { + guard let context = enforcedContext() else { return EmptyCancellable() } + // Resolved NOW, while the model is live: after removal the context is sealed + // and its root may be gone. + let access = context.rootParent.modelAccess + return SignalHandler( + cancellations: context.cancellations, + match: match, once: once, cancelPrevious: cancelPrevious, + modelName: typeDescription, + taskName: name ?? "\(function) @ \(fileAndLine.description)", + fileAndLine: fileAndLine, + host: access?.teardownWorkStore, access: access, + dependencies: context.capturedDependencies, + executor: _TestExecutorBox.current, + priority: priority, operation: perform + ) + } + + func _signal(_ key: CancellableKey?, to relation: ModelRelation) async { + guard let context = enforcedContext() else { return } + let handlers = context.reduceHierarchy(for: relation, observeParents: false, transform: \.self, into: [SignalHandler]()) { result, context in + let store = context.lock { context.cancellationsStore } + result += store?.registered(of: SignalHandler.self).filter { $0.matches(key) } ?? [] + } + let runs = handlers.compactMap { $0.start(.requested) } + for run in runs { + _ = try? await run.value + } + } +} + +final class SignalHandler: Cancellable, InternalCancellable, @unchecked Sendable { + enum Match { case any, key(CancellableKey), removalOnly } + + let id: Int + weak var cancellations: Cancellations? + let match: Match + let once: Bool + let cancelPrevious: Bool + let modelName: String + let taskName: String + let fileAndLine: FileAndLine + let host: Cancellations? + let access: ModelAccess? + let dependencies: DependencyValues + let executor: (any Sendable)? + let priority: TaskPriority? + let operation: @Sendable (SignalCause) async -> Void + + private let lock = NSLock() + private var hasRun = false + private var isUnregistered = false + private var tail: Task? + + init(cancellations: Cancellations, match: Match, once: Bool, cancelPrevious: Bool, modelName: String, taskName: String, fileAndLine: FileAndLine, host: Cancellations?, access: ModelAccess?, dependencies: DependencyValues, executor: (any Sendable)?, priority: TaskPriority?, operation: @escaping @Sendable (SignalCause) async -> Void) { + self.cancellations = cancellations + self.id = cancellations.nextId + self.match = match + self.once = once + self.cancelPrevious = cancelPrevious + self.modelName = modelName + self.taskName = taskName + self.fileAndLine = fileAndLine + self.host = host + self.access = access + self.dependencies = dependencies + self.executor = executor + self.priority = priority + self.operation = operation + cancellations.register(self) + } + + func matches(_ key: CancellableKey?) -> Bool { + switch match { + case .removalOnly: return false + case .any: return true + case .key(let own): return key == nil || key == own + } + } + + /// From the model's store. A sealed store is a removal → the final call. Otherwise + /// it is a user cancellation (`cancelAll(for:)`) → unregister only. + func onCancel() { + if cancellations?.isSealed ?? true { + // The harness's own end-of-test teardown: not the test's removal — skip. + guard access?.isInHarnessTeardown != true else { return } + _ = start(.removed) + } else { + lock { isUnregistered = true } + } + } + + func cancel() { + lock { isUnregistered = true } + _ = cancellations?.unregister(id) + } + + @discardableResult + func cancel(for key: some Hashable & Sendable, cancelInFlight: Bool) -> Self { + cancellations?.cancel(self, for: key, cancelInFlight: cancelInFlight) + return self + } + + /// Starts one run (serialized behind, or cancelling, the previous one). `nil` when + /// the handler is unregistered or `once` and already run. + func start(_ cause: SignalCause) -> Task? { + let operation = self.operation + let dependencies = self.dependencies + let access = self.access + let executor = self.executor + let priority = self.priority + let taskName = self.taskName + let started = LockIsolated(false) + + // Read the previous run AND publish this one in the same critical section: + // two concurrent `signal`s must see each other, or both run unserialized. + // Spawning inside the lock is safe — neither `Task.init` nor the host store's + // `register` (never sealed) calls back into this handler. + let (task, previous): (Task?, Task?) = lock { + if isUnregistered { return (nil, nil) } + if once && hasRun { return (nil, nil) } + hasRun = true + let previous = tail + let serialized = cancelPrevious ? nil : previous + + let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in + let body = { @Sendable () async throws -> Void in + defer { onDone() } + _ = try? await serialized?.value + await DependencyValues.$_current.withValue(dependencies) { + started.setValue(true) + access?.taskBodyStarted() + await operation(cause) + } + } + #if canImport(Dispatch) + if #available(macOS 15.0, iOS 18.0, tvOS 18.0, watchOS 11.0, *), + let exec = executor as? _DrainTestExecutor { + return Task(executorPreference: exec, priority: priority, operation: body) + } + #endif + return Task(name: taskName, priority: priority, operation: body) + } + + let task: Task? + if let host { + task = TaskCancellable(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: host, hasStartedRunningBox: started, task: makeTask).underlyingTask + } else { + task = makeTask {} + } + tail = task + return (task, previous) + } + if cancelPrevious { previous?.cancel() } + return task + } +} diff --git a/Sources/SwiftModel/ModelNode+Teardown.swift b/Sources/SwiftModel/ModelNode+Teardown.swift deleted file mode 100644 index ea9f8545..00000000 --- a/Sources/SwiftModel/ModelNode+Teardown.swift +++ /dev/null @@ -1,66 +0,0 @@ -import Foundation -import Dependencies -import ConcurrencyExtras - -public extension ModelNode { - /// SPIKE — async work that starts when this model is deactivated and is allowed to - /// outlive it (an audio fade-out, a final analytics flush). - /// - /// Unlike starting a `Task` from `onCancel`, the work is registered while the model - /// is still live, so it cannot be dropped by a teardown that has already sealed the - /// tree. It captures this model's dependencies at registration and runs with them. - /// - /// The model is gone while `operation` runs: capture the values you need up front, - /// and don't touch `node`. The work must tolerate cancellation. - /// - /// - Production: a plain task, owned by no model. - /// - `.modelTesting`: hosted by the test — it runs on the test's executor, `settle()` - /// waits for it, and work still running at the end of the test is reported as an - /// active task. - @discardableResult - func onTeardown(_ name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, operation: @escaping @Sendable () async -> Void) -> Cancellable { - guard let context = enforcedContext() else { return EmptyCancellable() } - - let fileAndLine = FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column) - let taskName = name ?? "\(function) @ \(fileAndLine.description)" - let modelName = typeDescription - // Everything the work needs is resolved NOW, while the model is live: after - // removal the context is sealed and its root may be gone. - let dependencies = context.capturedDependencies - let executor = _TestExecutorBox.current - let access = context.rootParent.modelAccess - let store = access?.teardownWorkStore - - return AnyCancellable(cancellations: context.cancellations) { - _startTeardownWork(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, store: store, access: access, dependencies: dependencies, executor: executor, priority: priority, operation: operation) - } - } -} - -private func _startTeardownWork(modelName: String, taskName: String, fileAndLine: FileAndLine, store: Cancellations?, access: ModelAccess?, dependencies: DependencyValues, executor: (any Sendable)?, priority: TaskPriority?, operation: @escaping @Sendable () async -> Void) { - let hasStartedRunningBox = LockIsolated(false) - let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in - let body = { @Sendable () async throws -> Void in - await DependencyValues.$_current.withValue(dependencies) { - defer { onDone() } - hasStartedRunningBox.setValue(true) - access?.taskBodyStarted() - await operation() - } - } - #if canImport(Dispatch) - if #available(macOS 15.0, iOS 18.0, tvOS 18.0, watchOS 11.0, *), - let exec = executor as? _DrainTestExecutor { - return Task(executorPreference: exec, priority: priority, operation: body) - } - #endif - return Task(name: taskName, priority: priority, operation: body) - } - - guard let store else { - // Production: nobody to report to — just run it. - _ = makeTask {} - return - } - _ = TaskCancellable(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: store, hasStartedRunningBox: hasStartedRunningBox, task: makeTask) -} diff --git a/Sources/SwiftModel/Testing/ModelTestingSupport.swift b/Sources/SwiftModel/Testing/ModelTestingSupport.swift index d0d041af..11f3e99a 100644 --- a/Sources/SwiftModel/Testing/ModelTestingSupport.swift +++ b/Sources/SwiftModel/Testing/ModelTestingSupport.swift @@ -323,9 +323,9 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch // The seal makes that drain unnecessary. tester.access.context.sealRecursively() - // SPIKE: teardown work already running now was started by the test itself. - let midTestTeardownWork = tester.access.teardownWork.registeredIDs - tester.access.lock { tester.access.reportableTeardownWork = midTestTeardownWork } + // SPIKE: from here on removals are the harness's, not the test's — don't start + // their removal calls (see `TestAccess.isHarnessTeardown`). + tester.access.isHarnessTeardown.setValue(true) // Phase 2: Cancel all currently-registered onActivate tasks. tester.access.context.cancelAllRecursively(for: ContextCancellationKey.onActivate) diff --git a/Tests/SwiftModelTests/SignalTests.swift b/Tests/SwiftModelTests/SignalTests.swift new file mode 100644 index 00000000..e0913f68 --- /dev/null +++ b/Tests/SwiftModelTests/SignalTests.swift @@ -0,0 +1,213 @@ +import Testing +@testable import SwiftModel +import ConcurrencyExtras +import Clocks +import Foundation + +// SPIKE acceptance tests for `onSignal` / `signal`. + +private enum Lifecycle: Hashable, Sendable { case flush, leave } + +@Model private struct Reporter { + let calls: TestProbe + + func onActivate() { + let calls = calls + node.onSignal(Lifecycle.flush) { cause in calls("flush \(cause)") } + } +} + +@Model private struct Player { + let calls: TestProbe + + func onActivate() { + let calls = calls + node.onSignal(Lifecycle.leave, once: true) { cause in calls("leave \(cause)") } + } +} + +@Model private struct AnyListener { + let calls: TestProbe + + func onActivate() { + let calls = calls + node.onSignal { cause in calls("any \(cause)") } + } +} + +@Model private struct Show { + var reporter: Reporter? + var player: Player? + var listener: AnyListener? +} + +@Suite(.modelTesting) +struct SignalTests { + @Test func signalReachesDescendantsAndIsAwaited() async { + let calls = TestProbe() + let show = Show(reporter: Reporter(calls: calls)).withAnchor() + + await show.node.signal(Lifecycle.flush) + #expect(calls.count == 1) // already done when signal returns + await expect(calls.wasCalled(with: "flush requested")) + } + + @Test func keysSelectHandlers() async { + let calls = TestProbe() + let show = Show(reporter: Reporter(calls: calls), player: Player(calls: calls), listener: AnyListener(calls: calls)).withAnchor() + + await show.node.signal(Lifecycle.leave) + await expect { + calls.wasCalled(with: "leave requested") + calls.wasCalled(with: "any requested") + } + } + + @Test func unkeyedSignalReachesEveryHandler() async { + let calls = TestProbe() + let show = Show(reporter: Reporter(calls: calls), player: Player(calls: calls)).withAnchor() + + await show.node.signal() + await expect { + calls.wasCalled(with: "flush requested") + calls.wasCalled(with: "leave requested") + } + } + + @Test func signalIsRepeatable() async { + let calls = TestProbe() + let show = Show(reporter: Reporter(calls: calls)).withAnchor() + + await show.node.signal(Lifecycle.flush) + await show.node.signal(Lifecycle.flush) + await expect { + calls.wasCalled(with: "flush requested") + calls.wasCalled(with: "flush requested") + } + } + + @Test func removalGivesTheFinalCall() async { + let calls = TestProbe() + let show = Show(reporter: Reporter(calls: calls)).withAnchor() + + show.reporter = nil + await expect { + show.reporter == nil + calls.wasCalled(with: "flush removed") + } + } + + @Test func onceRunsOnlyOnceAcrossSignalAndRemoval() async { + let calls = TestProbe() + let show = Show(player: Player(calls: calls)).withAnchor() + + await show.node.signal(Lifecycle.leave) + await show.node.signal(Lifecycle.leave) + show.player = nil + await settle(resetting: .off) + await expect { + show.player == nil + calls.wasCalled(with: "leave requested") // exactly once (exhaustive) + } + } + + @Test func reachAncestors() async { + let calls = TestProbe() + let parent = SignalParent(calls: calls).withAnchor() + + await parent.child.node.signal(Lifecycle.flush, to: .ancestors) + await expect(calls.wasCalled(with: "parent flush requested")) + } + + @Test func cancelUnregisters() async { + let calls = TestProbe() + let model = Cancelling(calls: calls).withAnchor() + + model.unregister() + await model.node.signal(Lifecycle.flush) + await settle() + #expect(calls.count == 0) // not on the signal, and (at end of test) not on removal + } + + @Test func runsOfOneHandlerAreSerialized() async { + let log = LockIsolated<[String]>([]) + let model = Serial(log: log, cancelPrevious: false).withAnchor() + + async let a: Void = model.node.signal(Lifecycle.flush) + async let b: Void = model.node.signal(Lifecycle.flush) + _ = await (a, b) + #expect(log.value == ["begin", "end", "begin", "end"], "\(log.value)") + } + + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func cancelPreviousCancelsTheRunningOne() async { + let log = LockIsolated<[String]>([]) + let model = Parking(log: log).withAnchor { $0.continuousClock = TestClock() } + + let first = Task { await model.node.signal(Lifecycle.flush) } + await settle() // run 1 parked on the frozen clock + await model.node.signal(Lifecycle.flush) // run 2 cancels it + await first.value + #expect(log.value == ["park", "cancelled", "run 2"]) + } +} + +@Model private struct Parking { + let log: LockIsolated<[String]> + + func onActivate() { + let log = log + let clock = node.continuousClock + let runs = LockIsolated(0) + node.onSignal(Lifecycle.flush, cancelPrevious: true) { cause in + guard cause == .requested else { return } + let run = runs.withValue { $0 += 1; return $0 } + guard run == 1 else { log.withValue { $0.append("run \(run)") }; return } + log.withValue { $0.append("park") } + do { try await clock.sleep(for: .seconds(1)) } catch { log.withValue { $0.append("cancelled") } } + } + } +} + +@Model private struct SignalChild {} + +@Model private struct SignalParent { + let calls: TestProbe + var child = SignalChild() + + func onActivate() { + let calls = calls + node.onSignal(Lifecycle.flush) { cause in calls("parent flush \(cause)") } + } +} + +@Model private struct Cancelling { + let calls: TestProbe + let registration = LockIsolated<(any Cancellable)?>(nil) + + func onActivate() { + let calls = calls + registration.setValue(node.onSignal(Lifecycle.flush) { cause in calls("flush \(cause)") }) + } + + func unregister() { registration.value?.cancel() } +} + +@Model private struct Serial { + let log: LockIsolated<[String]> + let cancelPrevious: Bool + + func onActivate() { + let log = log + node.onSignal(Lifecycle.flush, cancelPrevious: cancelPrevious) { cause in + guard cause == .requested else { return } + log.withValue { $0.append("begin") } + do { + try await Task.sleep(nanoseconds: 50_000_000) + log.withValue { $0.append("end") } + } catch { + log.withValue { $0.append("cancelled") } + } + } + } +} diff --git a/Tests/SwiftModelTests/TeardownWorkTests.swift b/Tests/SwiftModelTests/TeardownWorkTests.swift index e930f099..e3bc9aeb 100644 --- a/Tests/SwiftModelTests/TeardownWorkTests.swift +++ b/Tests/SwiftModelTests/TeardownWorkTests.swift @@ -78,11 +78,10 @@ struct TeardownWorkTests { #expect(reporter.messages.contains { $0.contains("Active task 'fade' of `FadingPlayer` still running") }) } - // Removed by the harness's own end-of-test teardown: the fade is started (not - // dropped), but — like `onActivate` tasks — cancelled after cleanup rather than - // reported, even when it parks on a clock nobody advances. + // Removed by the harness's own end-of-test teardown: the test didn't cause that + // removal, so the fade is not started (and so neither runs nor gets reported). @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) - @Test func fadeStartedByEndOfTestTeardownIsCancelledNotReported() async { + @Test func fadeIsNotStartedByEndOfTestTeardown() async { let reporter = CapturingIssueReporter() let events = TestProbe() await withIssueReporters([reporter]) { @@ -92,9 +91,7 @@ struct TeardownWorkTests { } } } - // Cancellation reaches the parked sleep asynchronously. - try? await waitUntil(events.count == 1) - #expect(events.values.map { "\($0)" } == ["cancelled at 1"]) + #expect(events.count == 0) #expect(reporter.messages.isEmpty, "\(reporter.messages)") } } @@ -106,8 +103,10 @@ struct TeardownWorkReleaseTests { @Test func fadeRunsWhenWholeTreeIsReleased() async throws { let events = TestProbe() await waitUntilRemoved { + // A real clock: `ImmediateClock.sleep` hops through detached `.background` + // tasks (`megaYield`), which starve under parallel load. PlayerHost(player: FadingPlayer(events: events)).withAnchor { - $0.continuousClock = ImmediateClock() + $0.continuousClock = ContinuousClock() } } try await waitUntil(events.values.map { "\($0)" } == ["stopped"]) From 24ea8cae5388d7f1308a72b3c511bb84905c0974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 16:11:03 +0200 Subject: [PATCH 3/6] Signals: cancelPrevious waits for the cancelled run to unwind Runs of one handler never overlap, matching forEach(cancelPrevious:). CI on Linux and macOS serial caught the new run starting before the cancelled one had finished. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/SwiftModel/ModelNode+Signal.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftModel/ModelNode+Signal.swift b/Sources/SwiftModel/ModelNode+Signal.swift index caa95d40..cbff0c4d 100644 --- a/Sources/SwiftModel/ModelNode+Signal.swift +++ b/Sources/SwiftModel/ModelNode+Signal.swift @@ -23,7 +23,8 @@ public extension ModelNode { /// The handler also gets exactly one final call with `.removed` when this model is /// removed (unless `once` and it already ran). Runs of one handler are serialized /// (a new request waits for the running one), or with `cancelPrevious` the new one - /// cancels it. Different handlers always run concurrently. + /// cancels it and starts once it has unwound. Different handlers always run + /// concurrently. /// /// Cancelling the returned `Cancellable` (or `cancelAll(for:)` on a key it was /// registered under) unregisters the handler: it won't run again, not even on removal. @@ -184,7 +185,10 @@ final class SignalHandler: Cancellable, InternalCancellable, @unchecked Sendable if once && hasRun { return (nil, nil) } hasRun = true let previous = tail - let serialized = cancelPrevious ? nil : previous + // Always wait for the previous run — with `cancelPrevious` it is cancelled + // first, but runs of one handler never overlap (as `forEach(cancelPrevious:)` + // starts the next body only after the previous one has fully unwound). + let serialized = previous let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in let body = { @Sendable () async throws -> Void in From 337594392578bedb0a5f26642a67e516b320c487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 17:44:34 +0200 Subject: [PATCH 4/6] Signals: propagate caller cancellation; run end-of-test removal calls unchecked - Cancelling the task that called signal() cancels the runs that call started, like a task group, so a deadline wrapped around a signal reaches the handlers. - Removal calls caused by the harness's own end-of-test teardown are no longer skipped: they start after the exhaustion check (neither checked nor reported), are driven until quiet, and whatever is still parked is cancelled. 'Let the model go at scope exit, then assert its cleanup ran' works again. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/SwiftModel/Internal/ModelAccess.swift | 7 +++-- Sources/SwiftModel/Internal/TestAccess.swift | 31 +++++++++++++++---- Sources/SwiftModel/ModelNode+Signal.swift | 21 +++++++++---- .../Testing/ModelTestingSupport.swift | 17 ++++++++-- Tests/SwiftModelTests/SignalTests.swift | 30 ++++++++++++++++++ Tests/SwiftModelTests/TeardownWorkTests.swift | 11 ++++--- 6 files changed, 95 insertions(+), 22 deletions(-) diff --git a/Sources/SwiftModel/Internal/ModelAccess.swift b/Sources/SwiftModel/Internal/ModelAccess.swift index 829d4643..246d7837 100644 --- a/Sources/SwiftModel/Internal/ModelAccess.swift +++ b/Sources/SwiftModel/Internal/ModelAccess.swift @@ -145,9 +145,10 @@ class ModelAccess: ModelAccessReference, @unchecked Sendable { /// to `settle()` and the end-of-test task check after the model is gone. var teardownWorkStore: Cancellations? { nil } - /// SPIKE: `true` while the test harness tears down the model tree at the end of a - /// test — removal calls are skipped then. Always `false` in production. - var isInHarnessTeardown: Bool { false } + /// SPIKE: while the test harness tears the model tree down at the end of a test, + /// removal calls are deferred until after the exhaustion check (so they are neither + /// checked nor reported). Returns `true` if `start` was deferred. Production: `false`. + func deferRemovalCall(_ start: @escaping @Sendable () -> Void) -> Bool { false } /// Records that a reactive body (`node.forEach` / `node.onChange`) delivered /// an element, keyed by its source location. Powers `settle()`'s runaway diff --git a/Sources/SwiftModel/Internal/TestAccess.swift b/Sources/SwiftModel/Internal/TestAccess.swift index d71cc9ac..f0070588 100644 --- a/Sources/SwiftModel/Internal/TestAccess.swift +++ b/Sources/SwiftModel/Internal/TestAccess.swift @@ -523,12 +523,31 @@ final class TestAccess: ModelAccess, @unchecked Sendable { let teardownWork = Cancellations() override var teardownWorkStore: Cancellations? { teardownWork } - /// Set once the harness starts its own end-of-test teardown. Removal calls - /// (`onSignal` final call, `onTeardown`) are NOT started from then on: the test - /// didn't cause that removal, so its work is not the test's concern — the same way - /// `onActivate` tasks are cancelled rather than reported. - let isHarnessTeardown = LockIsolated(false) - override var isInHarnessTeardown: Bool { isHarnessTeardown.value } + /// Removal calls (`onSignal` final call, `onTeardown`) caused by the harness's own + /// end-of-test teardown. They start only AFTER the exhaustion check — the test didn't + /// trigger that removal, so its work is neither checked nor reported — and are then + /// driven until quiet; whatever is still parked (e.g. on a frozen clock) is cancelled. + /// `nil` = not in harness teardown. + private let deferredRemovals = LockIsolated<[@Sendable () -> Void]?>(nil) + + func beginHarnessTeardown() { + deferredRemovals.setValue([]) + } + + func takeDeferredRemovals() -> [@Sendable () -> Void] { + deferredRemovals.withValue { pending in + defer { pending = [] } + return pending ?? [] + } + } + + override func deferRemovalCall(_ start: @escaping @Sendable () -> Void) -> Bool { + deferredRemovals.withValue { pending in + guard pending != nil else { return false } + pending!.append(start) + return true + } + } /// Pending-start across the model tree AND hosted teardown work. var hasPendingStartWork: Bool { diff --git a/Sources/SwiftModel/ModelNode+Signal.swift b/Sources/SwiftModel/ModelNode+Signal.swift index cbff0c4d..dbfe3c85 100644 --- a/Sources/SwiftModel/ModelNode+Signal.swift +++ b/Sources/SwiftModel/ModelNode+Signal.swift @@ -7,7 +7,9 @@ import ConcurrencyExtras // call runs AFTER the model is gone, so it must not be hosted by the model: in // production it is a plain task, in `.modelTesting` it is hosted by the test harness // (runs on the test's executor, seen by `settle()`, reported if still running at the -// end of the test when the test itself triggered it). +// end of the test when the test itself triggered it). Removals caused by the harness's +// own end-of-test teardown run after the exhaustion check, unchecked, and are driven +// until quiet; work still parked then is cancelled. /// Why a signal handler is running. public enum SignalCause: Sendable, Equatable { @@ -50,7 +52,8 @@ public extension ModelNode { } /// Runs every handler registered for `key` on the models `relation` reaches, - /// concurrently, and returns when all of those runs have finished. + /// concurrently, and returns when all of those runs have finished. Cancelling the + /// calling task cancels those runs. func signal(_ key: some Hashable & Sendable, to relation: ModelRelation = [.self, .descendants]) async { await _signal(CancellableKey(key: key), to: relation) } @@ -87,8 +90,14 @@ private extension ModelNode { result += store?.registered(of: SignalHandler.self).filter { $0.matches(key) } ?? [] } let runs = handlers.compactMap { $0.start(.requested) } - for run in runs { - _ = try? await run.value + // Structured like a task group: cancelling the caller cancels the runs this call + // started (e.g. a deadline wrapped around the signal). + await withTaskCancellationHandler { + for run in runs { + _ = try? await run.value + } + } onCancel: { + for run in runs { run.cancel() } } } } @@ -146,8 +155,8 @@ final class SignalHandler: Cancellable, InternalCancellable, @unchecked Sendable /// it is a user cancellation (`cancelAll(for:)`) → unregister only. func onCancel() { if cancellations?.isSealed ?? true { - // The harness's own end-of-test teardown: not the test's removal — skip. - guard access?.isInHarnessTeardown != true else { return } + // The harness's own end-of-test teardown defers it past the exhaustion check. + if access?.deferRemovalCall({ [self] in _ = self.start(.removed) }) == true { return } _ = start(.removed) } else { lock { isUnregistered = true } diff --git a/Sources/SwiftModel/Testing/ModelTestingSupport.swift b/Sources/SwiftModel/Testing/ModelTestingSupport.swift index 11f3e99a..42d8f71c 100644 --- a/Sources/SwiftModel/Testing/ModelTestingSupport.swift +++ b/Sources/SwiftModel/Testing/ModelTestingSupport.swift @@ -323,9 +323,9 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch // The seal makes that drain unnecessary. tester.access.context.sealRecursively() - // SPIKE: from here on removals are the harness's, not the test's — don't start - // their removal calls (see `TestAccess.isHarnessTeardown`). - tester.access.isHarnessTeardown.setValue(true) + // SPIKE: from here on removals are the harness's, not the test's — their removal + // calls are deferred past the exhaustion check (see `TestAccess.deferredRemovals`). + tester.access.beginHarnessTeardown() // Phase 2: Cancel all currently-registered onActivate tasks. tester.access.context.cancelAllRecursively(for: ContextCancellationKey.onActivate) @@ -338,14 +338,25 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch tester.access.checkExhaustion(at: fileAndLine, includeUpdates: false, checkTasks: true) tester.access.context.onRemoval() + + // SPIKE: now run the harness teardown's removal calls — unchecked — until quiet, + // so "let the model go at scope exit, then assert its cleanup ran" works; cancel + // whatever is still parked (e.g. on a clock nobody advances). + let deferred = tester.access.takeDeferredRemovals() + for start in deferred { start() } + if !deferred.isEmpty { + _ = await tester.access.waitUntilSettled(cleanup: true, at: fileAndLine) + } tester.access.teardownWork.cancelAll() } package func cancelAndCleanup() { // Mark tester so its deinit skips cleanup — we are running it here instead. tester.cleanupHandledExternally = true + tester.access.beginHarnessTeardown() // SPIKE: cancelled test — deferred removal calls are dropped tester.access.context.cancelAllRecursively(for: ContextCancellationKey.onActivate) tester.access.context.onRemoval() + tester.access.teardownWork.cancelAll() } package func waitForTeardown() async { diff --git a/Tests/SwiftModelTests/SignalTests.swift b/Tests/SwiftModelTests/SignalTests.swift index e0913f68..d6ea0516 100644 --- a/Tests/SwiftModelTests/SignalTests.swift +++ b/Tests/SwiftModelTests/SignalTests.swift @@ -3,6 +3,7 @@ import Testing import ConcurrencyExtras import Clocks import Foundation +import IssueReporting // SPIKE acceptance tests for `onSignal` / `signal`. @@ -139,6 +140,35 @@ struct SignalTests { #expect(log.value == ["begin", "end", "begin", "end"], "\(log.value)") } + // "Let the model go at scope exit, then assert its cleanup ran": the harness's own + // teardown runs the final call after the exhaustion check — unchecked, so the probe + // call isn't an unasserted-probe failure — and drives it to completion. + @Test func finalCallAtScopeExitRunsUnchecked() async { + let reporter = CapturingIssueReporter() + let calls = TestProbe() + await withIssueReporters([reporter]) { + await withModelTesting { + _ = Show(reporter: Reporter(calls: calls)).withAnchor() + } + } + #expect(calls.values.map { "\($0)" } == ["flush removed"]) + #expect(reporter.messages.isEmpty, "\(reporter.messages)") + } + + // Cancelling the caller of `signal` cancels the runs it started (a deadline wrapped + // around a signal must reach the handlers). + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func cancellingTheCallerCancelsItsRuns() async { + let log = LockIsolated<[String]>([]) + let model = Parking(log: log).withAnchor { $0.continuousClock = TestClock() } + + let caller = Task { await model.node.signal(Lifecycle.flush) } + await settle() // run 1 parked on the frozen clock + caller.cancel() + await caller.value + #expect(log.value == ["park", "cancelled"]) + } + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) @Test func cancelPreviousCancelsTheRunningOne() async { let log = LockIsolated<[String]>([]) diff --git a/Tests/SwiftModelTests/TeardownWorkTests.swift b/Tests/SwiftModelTests/TeardownWorkTests.swift index e3bc9aeb..aca81a41 100644 --- a/Tests/SwiftModelTests/TeardownWorkTests.swift +++ b/Tests/SwiftModelTests/TeardownWorkTests.swift @@ -78,10 +78,11 @@ struct TeardownWorkTests { #expect(reporter.messages.contains { $0.contains("Active task 'fade' of `FadingPlayer` still running") }) } - // Removed by the harness's own end-of-test teardown: the test didn't cause that - // removal, so the fade is not started (and so neither runs nor gets reported). + // Removed by the harness's own end-of-test teardown: the fade starts after the + // exhaustion check (unchecked, unreported), runs until quiet, and — parked on a + // clock nobody advances — is then cancelled. @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) - @Test func fadeIsNotStartedByEndOfTestTeardown() async { + @Test func fadeStartedByEndOfTestTeardownIsCancelledNotReported() async { let reporter = CapturingIssueReporter() let events = TestProbe() await withIssueReporters([reporter]) { @@ -91,7 +92,9 @@ struct TeardownWorkTests { } } } - #expect(events.count == 0) + // Cancellation reaches the parked sleep asynchronously. + try? await waitUntil(events.count == 1) + #expect(events.values.map { "\($0)" } == ["cancelled at 1"]) #expect(reporter.messages.isEmpty, "\(reporter.messages)") } } From ba5941dc94aee802b8cf7e73ac20f9fabe032321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 18:27:16 +0200 Subject: [PATCH 5/6] Signals: scope exit waits for cancelled teardown work to unwind Cleanup in a cancelled run (defer { stop(); release() }) has now happened before withModelTesting returns. The wait is bounded by the drive reaching quiescence, not by wall-clock time, so a run that ignores cancellation can't hang the test. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/SwiftModel/Internal/TestAccess.swift | 21 ++++++++++++ .../Testing/ModelTestingSupport.swift | 2 +- Tests/SwiftModelTests/TeardownWorkTests.swift | 32 +++++++++++++++++-- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftModel/Internal/TestAccess.swift b/Sources/SwiftModel/Internal/TestAccess.swift index f0070588..ab674e35 100644 --- a/Sources/SwiftModel/Internal/TestAccess.swift +++ b/Sources/SwiftModel/Internal/TestAccess.swift @@ -530,6 +530,27 @@ final class TestAccess: ModelAccess, @unchecked Sendable { /// `nil` = not in harness teardown. private let deferredRemovals = LockIsolated<[@Sendable () -> Void]?>(nil) + /// Cancels hosted teardown work still running at the end of a test and waits for it + /// to unwind, so cleanup in a cancelled run (`defer { stop(); release() }`) has + /// happened before `withModelTesting` returns. Evidence-based bound: stops waiting + /// once the drive reaches quiescence — a run that ignores cancellation and parks + /// again can't hang the test. + func cancelTeardownWorkAndAwaitUnwind(at fileAndLine: FileAndLine) async { + let runs = teardownWork.registered(of: TaskCancellable.self).compactMap(\.underlyingTask) + teardownWork.cancelAll() + guard !runs.isEmpty else { return } + await withTaskGroup(of: Void.self) { group in + group.addTask { + for run in runs { _ = try? await run.value } + } + group.addTask { + _ = await self.waitUntilSettled(cleanup: true, at: fileAndLine) + } + await group.next() + group.cancelAll() + } + } + func beginHarnessTeardown() { deferredRemovals.setValue([]) } diff --git a/Sources/SwiftModel/Testing/ModelTestingSupport.swift b/Sources/SwiftModel/Testing/ModelTestingSupport.swift index 42d8f71c..a015a346 100644 --- a/Sources/SwiftModel/Testing/ModelTestingSupport.swift +++ b/Sources/SwiftModel/Testing/ModelTestingSupport.swift @@ -347,7 +347,7 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch if !deferred.isEmpty { _ = await tester.access.waitUntilSettled(cleanup: true, at: fileAndLine) } - tester.access.teardownWork.cancelAll() + await tester.access.cancelTeardownWorkAndAwaitUnwind(at: fileAndLine) } package func cancelAndCleanup() { diff --git a/Tests/SwiftModelTests/TeardownWorkTests.swift b/Tests/SwiftModelTests/TeardownWorkTests.swift index aca81a41..075ba72a 100644 --- a/Tests/SwiftModelTests/TeardownWorkTests.swift +++ b/Tests/SwiftModelTests/TeardownWorkTests.swift @@ -33,6 +33,19 @@ import IssueReporting } } +@Model private struct ReleasingPlayer { + let released: LockIsolated + + func onActivate() { + let clock = node.continuousClock + let released = released + node.onTeardown("fade") { + defer { released.setValue(true) } // stop/release even when cancelled + try? await clock.sleep(for: .seconds(1)) + } + } +} + @Model private struct PlayerHost { var player: FadingPlayer? } @@ -92,13 +105,26 @@ struct TeardownWorkTests { } } } - // Cancellation reaches the parked sleep asynchronously. - try? await waitUntil(events.count == 1) - #expect(events.values.map { "\($0)" } == ["cancelled at 1"]) + #expect(events.values.map { "\($0)" } == ["cancelled at 1"]) // unwound before return #expect(reporter.messages.isEmpty, "\(reporter.messages)") } } +extension TeardownWorkTests { + // A fade parked at scope exit is cancelled, and its cleanup has run by the time + // `withModelTesting` returns — no polling needed. + @available(macOS 13, iOS 16, tvOS 16, watchOS 9, *) + @Test func cancelledFadeHasUnwoundWhenScopeReturns() async { + let released = LockIsolated(false) + await withModelTesting { + _ = ReleasingPlayer(released: released).withAnchor { + $0.continuousClock = TestClock() + } + } + #expect(released.value) + } +} + // Production path (no harness): the whole tree is released, and the fade — which a // `node.task` started from `onCancel` would lose — still runs to completion. struct TeardownWorkReleaseTests { From a4dc4bc4ea3a05467730ded891e2b00fb8a26490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5ns=20Bernhardt?= Date: Fri, 25 Sep 2026 19:01:27 +0200 Subject: [PATCH 6/6] =?UTF-8?q?Signals:=20finish=20for=20review=20?= =?UTF-8?q?=E2=80=94=20docs,=20CHANGELOG,=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Full DocC on onSignal/signal/onTeardown/SignalCause, SignalCause in the topic list, a Lifecycle guide section and testing notes in Docs/Testing.md, plus a contributor note in Docs/Contributing/TestInfrastructure.md. - CHANGELOG entry under [Unreleased]. - Remove spike markers; rename the harness store to signalWork; move SignalHandler to Internal/; put the TaskCancellable lock-order note where the hoist now happens. - The #83 teardown-registration issue now points at onTeardown. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 10 + Docs/Contributing/TestInfrastructure.md | 4 + Docs/Lifecycle.md | 37 ++- Docs/Testing.md | 15 + .../Documentation.docc/SwiftModel.md | 1 + .../SwiftModel/Internal/Cancellables.swift | 12 +- .../SwiftModel/Internal/Cancellations.swift | 4 +- Sources/SwiftModel/Internal/ModelAccess.swift | 19 +- .../SwiftModel/Internal/SignalHandler.swift | 140 ++++++++++ Sources/SwiftModel/Internal/TestAccess.swift | 23 +- Sources/SwiftModel/ModelNode+Signal.swift | 264 +++++++----------- .../Testing/ModelTestingSupport.swift | 12 +- Tests/SwiftModelTests/SignalTests.swift | 2 +- Tests/SwiftModelTests/TeardownWorkTests.swift | 2 +- 14 files changed, 345 insertions(+), 200 deletions(-) create mode 100644 Sources/SwiftModel/Internal/SignalHandler.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 18b30fc3..c0f74341 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ All notable changes are documented here. The format follows [Keep a Changelog](h ## [Unreleased] +### Added + +- **Signals: `onSignal` / `signal` / `onTeardown` — async work on request, and work that outlives its model.** `onCancel` runs synchronously during teardown, so it can't `await`. And a `Task` started from it is invisible to tests (and starves under parallel test load), while `node.task` can no longer start there — the whole removed subtree is already sealed. Apps ended up with raw `Task.detached` fallbacks and hand-built shutdown registries. + - `node.onSignal(key, once:cancelPrevious:) { cause in … }` registers an async handler. `await node.signal(key, to:)` reaches handlers like `send(_:to:)` reaches event listeners (default: the model and its descendants; `.ancestors` and `.dependencies` work too), runs them concurrently, and returns when they have finished. Signals are repeatable. + - Every handler also gets one final call when its model is removed, with `SignalCause.removed`, versus `.requested` for a signal. The final call runs after the model is gone, so handlers capture what they need. + - `node.onTeardown { … }` is the removal-only form, for things like fading out audio after a player's model is removed. + - Runs of one handler never overlap (`cancelPrevious` cancels the running one first). `once` runs a handler at most once in total. Cancelling the returned `Cancellable` unregisters the handler. Cancelling the caller of `signal` cancels the runs it started. + - Runs are never hosted by their own model: in production they are plain tasks, and under `.modelTesting` they run on the test's executor. `settle()` waits for them. A run the test caused that is still going at the end is reported as an active task. Removals caused by the harness's own end-of-test teardown run after the exhaustivity check (unchecked), and the scope waits for them, cancelling what is still parked, before it returns. + - Handlers live in each model's existing task registry, and `signal` reaches them through the same `reduceHierarchy` routing events use, so there's no new traversal. `SignalTests` and `TeardownWorkTests` cover reach, keys, repeatability, `once`, unregistering, serialization, cancellation, a fade stepping on a frozen `TestClock` after removal, whole-tree release outside the harness, and the end-of-test semantics. + ### Fixed - **A user `onCancel` handler that started new work during a sealed model's teardown drain was silently dropped.** `AnyContext.onRemoval` seals a model's `Cancellations` store, then drains it synchronously via `cancelAll()`/`cancelAll(for:)`. If one of the drained `onCancel` closures itself called `node.task { }`, a nested `node.onCancel { }`, or `forEach`, that registration landed on the already-sealed store: `Cancellations.register` cancelled it immediately before its body ever ran, with no signal — indistinguishable from the expected-silent case of a registration racing teardown from a different thread. `Cancellations.register` now checks a thread-local, `threadLocals.isDrainingCancellations`, set only around the synchronous drain in `cancelAll()`/`cancelAll(for:)`, and calls `reportIssue` (outside the lock) when a same-thread registration lands on a store sealed by the drain it's running inside of. A cross-thread race against teardown still stays silent, as intended — including a `Task { }` spawned from an `onCancel` handler that registers later: a thread-local, unlike a `@TaskLocal`, isn't inherited by that task. `OnCancelDuringTeardownRegistrationTests` is the regression coverage. diff --git a/Docs/Contributing/TestInfrastructure.md b/Docs/Contributing/TestInfrastructure.md index da4748be..908138d2 100644 --- a/Docs/Contributing/TestInfrastructure.md +++ b/Docs/Contributing/TestInfrastructure.md @@ -34,6 +34,10 @@ A per-test 30 s wall-clock cap is enforced by the `.modelTesting` trait. Hangs s **`SWIFT_MODEL_TIMEOUT_SCALE`** — multiplier on every test-infrastructure timeout: `expect` (5 s default), in-test `settle` (5 s), cleanup `settle` (25 s), trait cap (30 s), `waitUntil`'s drive backstop (120 s), the drive's termination ceiling (2× the trait ceiling), the meta-test bounds, and **every `waitUntil` call** (default 5 s and any explicit `timeout:` arg). Note the drive's *fail* verdicts are evidence-based, not wall-clock (runaway-fire bound; see `Docs/test-determinism-executor-drain.md` Update 27) — the scale only stretches backstops and budgets, never the discriminator. Defaults to `1.0` for fast local feedback. CI sets this to `3` so the `.deferential` `.background` QoS callbacks have wall-clock to actually fire on small parallel-saturated runners. Bump to 2–4 in any environment where you see meta-test or budget timeouts that aren't real bugs. Explicit `waitUntil(..., timeout: X)` is scaled too — that's deliberate, so individual tests don't need to know about CI tolerance. +## Signal-handler runs (`onSignal`, `onTeardown`) + +A handler's run is never hosted by its own model: the model may be removed mid-run, or already be gone for the final `.removed` call. Under `.modelTesting` runs are registered in `TestAccess.signalWork` (a `Cancellations` owned by the test, never sealed with the tree) and spawned on the executor captured when the handler was registered. So the drive and `settle()` see them (`hasPendingStartWork`), and `checkExhaustion(checkTasks:)` reports them. At scope exit, `beginHarnessTeardown()` makes `SignalHandler.onCancel` defer its removal call (`deferRemovalCall`). The deferred calls start after the exhaustion check and `onRemoval()`, get driven to quiescence, and then `cancelSignalWorkAndAwaitUnwind` cancels what's left and waits for it to unwind (bounded by the drive, not wall-clock). + ## `GlobalTickScheduler` (GTS) — settle's deadline source `Sources/SwiftModel/Internal/GlobalTickScheduler.swift` is the GCD-backed deadline scheduler that every wait primitive (`expect`, `settle`, `waitUntil`, the per-test trait cap) routes through. Key design points worth knowing before touching it: diff --git a/Docs/Lifecycle.md b/Docs/Lifecycle.md index 4c1d8a67..513508aa 100644 --- a/Docs/Lifecycle.md +++ b/Docs/Lifecycle.md @@ -97,7 +97,42 @@ node.cancellationContext(for: saveFlowID) { // group node.cancelAll(for: saveFlowID) // cancels both ``` -To **cancel-in-flight** — replace an ongoing operation each time a function is called — use `.cancelInFlight()` (id synthesised from the call site) or `.cancel(for: id, cancelInFlight: true)`. Nested work can join its parent's context with `.inheritCancellationContext()`, and `node.onCancel { … }` runs cleanup on cancellation. +To **cancel-in-flight** — replace an ongoing operation each time a function is called — use `.cancelInFlight()` (id synthesised from the call site) or `.cancel(for: id, cancelInFlight: true)`. Nested work can join its parent's context with `.inheritCancellationContext()`, and `node.onCancel { … }` runs cleanup on cancellation. For cleanup that has to `await`, see [Signals and work that outlives a model](#signals-and-work-that-outlives-a-model). + +### Signals and work that outlives a model + +`onCancel` runs synchronously while the model is being torn down, so it can't `await` anything. And a `Task` started from it is invisible to tests, while `node.task` can no longer start at that point. Work that has to finish *after* the model is gone — fading out audio, flushing a last analytics batch — goes in `onTeardown`: + +```swift +func onActivate() { + let player = player // capture what the work needs + let clock = node.continuousClock + node.onTeardown { + defer { player.stop() } // also when cancelled + await player.fadeOut(over: .seconds(1), on: clock) + } +} +``` + +When the work should also run **on request** — flush before leaving, save before syncing — register a signal handler instead. `signal` reaches handlers like `send` reaches event listeners (by default the model and its descendants), runs them all concurrently, and returns once they're done. Every handler also gets one final call when its model is removed: + +```swift +enum Lifecycle: Hashable, Sendable { case flush, leave } + +// ExperienceReporter +node.onSignal(Lifecycle.flush) { cause in + await reporter.flush(final: cause == .removed) +} + +// The leaving side: announce while the tree is live, then remove. +func leave() async { + await node.signal(Lifecycle.leave) + experience = nil + await node.signal(Lifecycle.flush) +} +``` + +The `cause` tells a handler whether its model is still live (`.requested`: use `node` as usual) or already gone (`.removed`: use only captured values). Runs of one handler never overlap: a new request waits for the running one, or with `cancelPrevious: true` cancels it first. `once: true` runs a handler at most once in total, on the first request or on removal. Cancelling the returned `Cancellable` unregisters the handler. Cancelling the task that called `signal` cancels the runs that call started, so a deadline around a signal reaches the handlers. ### Transactions diff --git a/Docs/Testing.md b/Docs/Testing.md index 0be4d6a3..15d4f24e 100644 --- a/Docs/Testing.md +++ b/Docs/Testing.md @@ -128,6 +128,21 @@ await clock.advance(by: .seconds(1)) await expect(model.secondsElapsed == 1) ``` +Prefer `TestClock` for anything that sleeps inside model work. `ImmediateClock` "sleeps" by awaiting detached background tasks, which run outside the test's scheduler and can starve when tests run in parallel. + +### Signals and teardown work + +`await node.signal(…)` returns once every handler it reached has finished, so you can assert on the result right after it. Work started by `onTeardown` or a handler's final `.removed` call runs on the test's executor even though its model is gone. `settle()` waits for it, and if the test removed the model and the work is still running when the test ends, it's reported as an active task. A fade parked on a `TestClock` steps forward as you advance the clock: + +```swift +host.player = nil // starts the player's onTeardown fade +await settle() // fade parked on its next sleep +await clock.advance(by: .seconds(1)) +await expect(events.wasCalled(with: "stopped")) +``` + +Models still alive when the test ends are removed by the test harness. Their final calls run *after* the exhaustivity check, so what they do is neither checked nor reported. The scope waits for them before returning and cancels whatever is still parked, such as a fade on a clock nobody advances. So a test can let a model go at the end of `withModelTesting` and then assert that its cleanup ran. + ### Refactor-resilient tests SwiftModel tests assert **final state**, not the sequence of actions or effects that produced it. There is no action enum to enumerate and no `send`/`receive` script to keep in sync — you call a method and assert the outcome: diff --git a/Sources/SwiftModel/Documentation.docc/SwiftModel.md b/Sources/SwiftModel/Documentation.docc/SwiftModel.md index f290a372..11e51e3f 100644 --- a/Sources/SwiftModel/Documentation.docc/SwiftModel.md +++ b/Sources/SwiftModel/Documentation.docc/SwiftModel.md @@ -107,6 +107,7 @@ import Testing ### Async Work - ``Cancellable`` +- ``SignalCause`` ### Observation diff --git a/Sources/SwiftModel/Internal/Cancellables.swift b/Sources/SwiftModel/Internal/Cancellables.swift index 989dab16..a86e2eef 100644 --- a/Sources/SwiftModel/Internal/Cancellables.swift +++ b/Sources/SwiftModel/Internal/Cancellables.swift @@ -73,7 +73,8 @@ final class TaskCancellable: Cancellable, InternalCancellable, @unchecked Sendab var hasStartedRunning: Bool { _hasStartedRunningBox.value } convenience init(modelName: String, taskName: String, fileAndLine: FileAndLine, context: AnyContext, hasStartedRunningBox: LockIsolated, task: @escaping @Sendable (@escaping @Sendable () -> Void) -> Task) { - // See the AB-BA note in the designated init: resolve the registry before any lock. + // `context.cancellations` is resolved HERE, before the designated init takes + // `lock` — see the AB-BA note there. self.init(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: context.cancellations, hasStartedRunningBox: hasStartedRunningBox, task: task) } @@ -81,7 +82,8 @@ final class TaskCancellable: Cancellable, InternalCancellable, @unchecked Sendab // Assigned before `cancellations.register(self)` below publishes this // instance to any settle thread — see `_hasStartedRunningBox`. self._hasStartedRunningBox = hasStartedRunningBox - // Resolve the registry ONCE, before `lock` is taken. `AnyContext.cancellations` + // The registry is resolved by the caller, before `lock` is taken (the convenience + // init evaluates `context.cancellations` up front). `AnyContext.cancellations` // acquires the per-context hierarchy lock (H); this instance's `lock` is T. // Evaluating `context.cancellations` *inside* `lock { }` — as the capture-list // expression below used to — orders this init T→H, while teardown runs H→T: @@ -95,9 +97,9 @@ final class TaskCancellable: Cancellable, InternalCancellable, @unchecked Sendab // which holds it across its entire body (`Context.transaction`), so the drain // still runs under H. Citing `onRemoval` alone gets this dismissed on review. // - // Hoisting is free — the value is needed on the first line anyway, so this - // takes and releases H exactly where it already did, just once. The ordering - // is now uniformly H-before-T and the cycle is gone by construction. + // Hoisting is free — the value is needed first anyway, so this takes and + // releases H exactly where it already did, just once. The ordering is now + // uniformly H-before-T and the cycle is gone by construction. // // Same family as the `reduceHierarchy` (#29) and `memoize` (#30) inversions: // whenever a leaf lock is held, do not evaluate anything that reaches a diff --git a/Sources/SwiftModel/Internal/Cancellations.swift b/Sources/SwiftModel/Internal/Cancellations.swift index 4b5d45d9..ba5f6687 100644 --- a/Sources/SwiftModel/Internal/Cancellations.swift +++ b/Sources/SwiftModel/Internal/Cancellations.swift @@ -36,7 +36,7 @@ final class Cancellations: @unchecked Sendable { lock { _sealed } } - /// Registered cancellables of a given type (SPIKE: signal handler lookup). + /// Registered cancellables of a given type (signal-handler lookup). func registered(of type: T.Type) -> [T] { lock { registered.values.compactMap { $0 as? T } } } @@ -64,7 +64,7 @@ final class Cancellations: @unchecked Sendable { let subject = (c as? TaskCancellable).map { "Task '\($0.taskName)' on `\($0.modelName)`" } ?? "A cancellable" - let message = "\(subject) was registered while a model is being deactivated (from an `onCancel` handler); it is cancelled immediately and never runs. Work that must outlive a model belongs to a model that outlives it (e.g. start it with the parent's `node.task`)." + let message = "\(subject) was registered while a model is being deactivated (from an `onCancel` handler); it is cancelled immediately and never runs. Register work that must run after removal while the model is live, with `node.onTeardown { … }` (or a signal handler's final call)." if let fileAndLine = (c as? TaskCancellable)?.fileAndLine { reportIssue(message, fileID: fileAndLine.fileID, filePath: fileAndLine.filePath, line: fileAndLine.line, column: fileAndLine.column) } else { diff --git a/Sources/SwiftModel/Internal/ModelAccess.swift b/Sources/SwiftModel/Internal/ModelAccess.swift index 246d7837..1911158e 100644 --- a/Sources/SwiftModel/Internal/ModelAccess.swift +++ b/Sources/SwiftModel/Internal/ModelAccess.swift @@ -139,15 +139,16 @@ class ModelAccess: ModelAccessReference, @unchecked Sendable { /// Default: no-op. `TestAccess` overrides to fire `_noteActivity`. func taskBodyStarted() {} - /// SPIKE (async teardown work): the store that hosts `node.onTeardown` work once - /// its model has been removed. `nil` in production — the work runs as a plain, - /// untracked task. `TestAccess` returns a store it owns, so the work stays visible - /// to `settle()` and the end-of-test task check after the model is gone. - var teardownWorkStore: Cancellations? { nil } - - /// SPIKE: while the test harness tears the model tree down at the end of a test, - /// removal calls are deferred until after the exhaustion check (so they are neither - /// checked nor reported). Returns `true` if `start` was deferred. Production: `false`. + /// The store that hosts signal-handler runs (`onSignal`, `onTeardown`). A run is + /// never hosted by its own model, which may be removed while it runs — or already + /// be gone, for the final `.removed` call. `nil` in production: runs are plain + /// tasks. `TestAccess` returns a store it owns, so runs stay visible to `settle()` + /// and the end-of-test task check after their model is gone. + var signalWorkStore: Cancellations? { nil } + + /// While the test harness tears the model tree down at the end of a test, removal + /// calls are deferred until after the exhaustion check (so they are neither checked + /// nor reported). Returns `true` if `start` was deferred. Production: `false`. func deferRemovalCall(_ start: @escaping @Sendable () -> Void) -> Bool { false } /// Records that a reactive body (`node.forEach` / `node.onChange`) delivered diff --git a/Sources/SwiftModel/Internal/SignalHandler.swift b/Sources/SwiftModel/Internal/SignalHandler.swift new file mode 100644 index 00000000..e285899a --- /dev/null +++ b/Sources/SwiftModel/Internal/SignalHandler.swift @@ -0,0 +1,140 @@ +import Foundation +import Dependencies +import ConcurrencyExtras + +/// One `onSignal` / `onTeardown` registration. +/// +/// Registered in its model's `Cancellations`, so removal reaches it through the store's +/// drain (`onCancel()` on a sealed store). Each run is a task hosted OUTSIDE the model — +/// the model may be removed while it runs, or already be gone for the final `.removed` +/// call: a plain task in production, `ModelAccess.signalWorkStore` under test. Runs of +/// one handler are chained through `tail`, so they never overlap. +final class SignalHandler: Cancellable, InternalCancellable, @unchecked Sendable { + enum Match { case any, key(CancellableKey), removalOnly } + + let id: Int + weak var cancellations: Cancellations? + let match: Match + let once: Bool + let cancelPrevious: Bool + let modelName: String + let taskName: String + let fileAndLine: FileAndLine + let host: Cancellations? + let access: ModelAccess? + let dependencies: DependencyValues + let executor: (any Sendable)? + let priority: TaskPriority? + let operation: @Sendable (SignalCause) async -> Void + + private let lock = NSLock() + private var hasRun = false + private var isUnregistered = false + private var tail: Task? + + init(cancellations: Cancellations, match: Match, once: Bool, cancelPrevious: Bool, modelName: String, taskName: String, fileAndLine: FileAndLine, host: Cancellations?, access: ModelAccess?, dependencies: DependencyValues, executor: (any Sendable)?, priority: TaskPriority?, operation: @escaping @Sendable (SignalCause) async -> Void) { + self.cancellations = cancellations + self.id = cancellations.nextId + self.match = match + self.once = once + self.cancelPrevious = cancelPrevious + self.modelName = modelName + self.taskName = taskName + self.fileAndLine = fileAndLine + self.host = host + self.access = access + self.dependencies = dependencies + self.executor = executor + self.priority = priority + self.operation = operation + cancellations.register(self) + } + + func matches(_ key: CancellableKey?) -> Bool { + switch match { + case .removalOnly: return false + case .any: return true + case .key(let own): return key == nil || key == own + } + } + + /// From the model's store. A sealed store is a removal → the final call. Otherwise + /// it is a user cancellation (`cancelAll(for:)`) → unregister only. + func onCancel() { + if cancellations?.isSealed ?? true { + // The harness's own end-of-test teardown defers it past the exhaustion check. + if access?.deferRemovalCall({ [self] in _ = self.start(.removed) }) == true { return } + _ = start(.removed) + } else { + lock { isUnregistered = true } + } + } + + func cancel() { + lock { isUnregistered = true } + _ = cancellations?.unregister(id) + } + + @discardableResult + func cancel(for key: some Hashable & Sendable, cancelInFlight: Bool) -> Self { + cancellations?.cancel(self, for: key, cancelInFlight: cancelInFlight) + return self + } + + /// Starts one run (serialized behind, or cancelling, the previous one). `nil` when + /// the handler is unregistered or `once` and already run. + func start(_ cause: SignalCause) -> Task? { + let operation = self.operation + let dependencies = self.dependencies + let access = self.access + let executor = self.executor + let priority = self.priority + let taskName = self.taskName + let started = LockIsolated(false) + + // Read the previous run AND publish this one in the same critical section: + // two concurrent `signal`s must see each other, or both run unserialized. + // Spawning inside the lock is safe — neither `Task.init` nor the host store's + // `register` (never sealed) calls back into this handler. + let (task, previous): (Task?, Task?) = lock { + if isUnregistered { return (nil, nil) } + if once && hasRun { return (nil, nil) } + hasRun = true + let previous = tail + // Always wait for the previous run — with `cancelPrevious` it is cancelled + // first, but runs of one handler never overlap (as `forEach(cancelPrevious:)` + // starts the next body only after the previous one has fully unwound). + let serialized = previous + + let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in + let body = { @Sendable () async throws -> Void in + defer { onDone() } + _ = try? await serialized?.value + await DependencyValues.$_current.withValue(dependencies) { + started.setValue(true) + access?.taskBodyStarted() + await operation(cause) + } + } + #if canImport(Dispatch) + if #available(macOS 15.0, iOS 18.0, tvOS 18.0, watchOS 11.0, *), + let exec = executor as? _DrainTestExecutor { + return Task(executorPreference: exec, priority: priority, operation: body) + } + #endif + return Task(name: taskName, priority: priority, operation: body) + } + + let task: Task? + if let host { + task = TaskCancellable(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: host, hasStartedRunningBox: started, task: makeTask).underlyingTask + } else { + task = makeTask {} + } + tail = task + return (task, previous) + } + if cancelPrevious { previous?.cancel() } + return task + } +} diff --git a/Sources/SwiftModel/Internal/TestAccess.swift b/Sources/SwiftModel/Internal/TestAccess.swift index ab674e35..de7ae280 100644 --- a/Sources/SwiftModel/Internal/TestAccess.swift +++ b/Sources/SwiftModel/Internal/TestAccess.swift @@ -517,11 +517,10 @@ final class TestAccess: ModelAccess, @unchecked Sendable { _noteActivity() } - /// SPIKE: hosts `node.onTeardown` work after its model is removed — see - /// `ModelAccess.teardownWorkStore`. Never sealed with the model tree; cancelled - /// after the final exhaustion check (and by `Cancellations.deinit`). - let teardownWork = Cancellations() - override var teardownWorkStore: Cancellations? { teardownWork } + /// Hosts signal-handler runs — see `ModelAccess.signalWorkStore`. Never sealed with + /// the model tree; cancelled (and awaited) when the test scope exits. + let signalWork = Cancellations() + override var signalWorkStore: Cancellations? { signalWork } /// Removal calls (`onSignal` final call, `onTeardown`) caused by the harness's own /// end-of-test teardown. They start only AFTER the exhaustion check — the test didn't @@ -530,14 +529,14 @@ final class TestAccess: ModelAccess, @unchecked Sendable { /// `nil` = not in harness teardown. private let deferredRemovals = LockIsolated<[@Sendable () -> Void]?>(nil) - /// Cancels hosted teardown work still running at the end of a test and waits for it + /// Cancels signal-handler runs still running at the end of a test and waits for it /// to unwind, so cleanup in a cancelled run (`defer { stop(); release() }`) has /// happened before `withModelTesting` returns. Evidence-based bound: stops waiting /// once the drive reaches quiescence — a run that ignores cancellation and parks /// again can't hang the test. - func cancelTeardownWorkAndAwaitUnwind(at fileAndLine: FileAndLine) async { - let runs = teardownWork.registered(of: TaskCancellable.self).compactMap(\.underlyingTask) - teardownWork.cancelAll() + func cancelSignalWorkAndAwaitUnwind(at fileAndLine: FileAndLine) async { + let runs = signalWork.registered(of: TaskCancellable.self).compactMap(\.underlyingTask) + signalWork.cancelAll() guard !runs.isEmpty else { return } await withTaskGroup(of: Void.self) { group in group.addTask { @@ -570,9 +569,9 @@ final class TestAccess: ModelAccess, @unchecked Sendable { } } - /// Pending-start across the model tree AND hosted teardown work. + /// Pending-start across the model tree AND hosted signal-handler runs. var hasPendingStartWork: Bool { - context.hasPendingStartTask || teardownWork.hasPendingStartTask + context.hasPendingStartTask || signalWork.hasPendingStartTask } // MARK: - Runaway diagnostic (settle-timeout) @@ -1687,7 +1686,7 @@ final class TestAccess: ModelAccess, @unchecked Sendable { func checkExhaustion(at fileAndLine: FileAndLine, includeUpdates: Bool, checkTasks: Bool = false, capturedUpdates: [PartialKeyPath: [ValueUpdate]]? = nil) { if checkTasks { - for info in context.activeTasks + teardownWork.activeTasks { + for info in context.activeTasks + signalWork.activeTasks { let taskWord = info.tasks.count == 1 ? "task" : "tasks" fail("Models of type `\(info.modelName)` have \(info.tasks.count) active \(taskWord) still running", for: .tasks, at: fileAndLine) diff --git a/Sources/SwiftModel/ModelNode+Signal.swift b/Sources/SwiftModel/ModelNode+Signal.swift index dbfe3c85..3134ef64 100644 --- a/Sources/SwiftModel/ModelNode+Signal.swift +++ b/Sources/SwiftModel/ModelNode+Signal.swift @@ -1,49 +1,103 @@ import Foundation -import Dependencies -import ConcurrencyExtras - -// SPIKE — signals: an awaitable, repeatable request that reaches related models (like -// `send`), plus a guaranteed final call when a handler's model is removed. The removal -// call runs AFTER the model is gone, so it must not be hosted by the model: in -// production it is a plain task, in `.modelTesting` it is hosted by the test harness -// (runs on the test's executor, seen by `settle()`, reported if still running at the -// end of the test when the test itself triggered it). Removals caused by the harness's -// own end-of-test teardown run after the exhaustion check, unchecked, and are driven -// until quiet; work still parked then is cancelled. /// Why a signal handler is running. +/// +/// See ``ModelNode/onSignal(_:once:cancelPrevious:name:function:priority:fileID:filePath:line:column:perform:)``. public enum SignalCause: Sendable, Equatable { - /// `signal(_:to:)` reached the handler. The model is live: `node` may be used. + /// A ``ModelNode/signal(_:to:)`` call reached the handler. The model is live: + /// `node`, its dependencies and its state can be used as usual. case requested - /// The handler's model was removed. The model is gone: use only captured values. + + /// The handler's model was removed; this is the handler's final call. The model is + /// gone: use only values captured when the handler was registered, and don't touch + /// `node`. case removed } public extension ModelNode { - /// Registers an async handler for `signal(key, to:)` requests. + /// Registers an async handler that runs when a ``signal(_:to:)`` for `key` reaches + /// this model, and one final time when the model is removed. + /// + /// Signals are for work a subtree does on request and must finish before the caller + /// continues — flush analytics before leaving, save before syncing — and that should + /// also happen, one last time, when the model goes away: + /// + /// ```swift + /// enum Lifecycle: Hashable, Sendable { case flush } + /// + /// func onActivate() { + /// let reporter = node.reporter // captured: the model may be gone + /// node.onSignal(Lifecycle.flush) { cause in + /// await reporter.flush(final: cause == .removed) + /// } + /// } /// - /// The handler also gets exactly one final call with `.removed` when this model is - /// removed (unless `once` and it already ran). Runs of one handler are serialized - /// (a new request waits for the running one), or with `cancelPrevious` the new one - /// cancels it and starts once it has unwound. Different handlers always run - /// concurrently. + /// // Elsewhere — returns once every reached handler has finished: + /// await session.node.signal(Lifecycle.flush) + /// ``` /// - /// Cancelling the returned `Cancellable` (or `cancelAll(for:)` on a key it was - /// registered under) unregisters the handler: it won't run again, not even on removal. + /// The final `.removed` call runs *after* the model is removed, so the handler must + /// not depend on `node` then: capture what it needs when registering. Work started + /// by it may outlive the model — an audio fade, a network flush — and is never + /// dropped by the removal. It should tolerate cancellation. + /// + /// - Runs of one handler never overlap: a new request waits for the running one, or + /// with `cancelPrevious` cancels it and starts once it has unwound. Different + /// handlers always run concurrently. + /// - With `once`, the handler runs at most one time in total — on the first request, + /// or on removal if no request reached it before. + /// - Cancelling the returned ``Cancellable`` (or ``cancelAll(for:)`` on a key it was + /// registered under) unregisters the handler: it won't run again, not even on + /// removal. + /// + /// In tests, runs execute on the test's executor, `settle()` + /// waits for them, and a run still going at the end of a test that removed its model + /// is reported as an active task. Removals caused by the test harness's own + /// end-of-test teardown run after the exhaustivity check (unchecked), and the scope + /// waits for them — cancelling whatever is still parked, e.g. on a `TestClock` nobody + /// advances — before it returns. + /// + /// - Parameters: + /// - key: The signal this handler answers to. + /// - once: Run at most one time in total. Defaults to `false`. + /// - cancelPrevious: A new run cancels the running one instead of waiting for it. + /// - name: Optional name for diagnostics; synthesized from the call site if omitted. + /// - perform: The work, given the ``SignalCause``. + /// - Returns: A ``Cancellable`` that unregisters the handler. @discardableResult func onSignal(_ key: some Hashable & Sendable, once: Bool = false, cancelPrevious: Bool = false, name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { _registerSignalHandler(match: .key(CancellableKey(key: key)), once: once, cancelPrevious: cancelPrevious, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column), perform: perform) } - /// Registers an async handler for every `signal` request (any key), plus the final - /// `.removed` call. See `onSignal(_:once:cancelPrevious:…)`. + /// Registers an async handler that runs for every ``signal(_:to:)`` reaching this + /// model, whatever its key, and one final time when the model is removed. + /// + /// Same semantics as ``onSignal(_:once:cancelPrevious:name:function:priority:fileID:filePath:line:column:perform:)``. @discardableResult func onSignal(once: Bool = false, cancelPrevious: Bool = false, name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { _registerSignalHandler(match: .any, once: once, cancelPrevious: cancelPrevious, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column), perform: perform) } - /// Async work that starts when this model is removed and may outlive it — a - /// removal-only handler (never reached by `signal`). Capture what you need up front. + /// Registers async work that starts when this model is removed and may outlive it. + /// + /// Use it for cleanup that takes time — fading out audio, releasing a resource + /// asynchronously — where ``onCancel(perform:)`` is too early to `await` and a `Task` + /// started from it would be invisible to tests: + /// + /// ```swift + /// func onActivate() { + /// let player = player // captured: the model will be gone + /// let clock = node.continuousClock + /// node.onTeardown { + /// defer { player.stop() } // also when cancelled + /// await player.fadeOut(over: .seconds(1), on: clock) + /// } + /// } + /// ``` + /// + /// It is a removal-only signal handler: never reached by ``signal(_:to:)``, run + /// exactly once on removal. Capture what the work needs; don't touch `node` in it. + /// Cancelling the returned ``Cancellable`` unregisters it. @discardableResult func onTeardown(_ name: String? = nil, function: StaticString = #function, priority: TaskPriority? = nil, fileID: StaticString = #fileID, filePath: StaticString = #filePath, line: UInt = #line, column: UInt = #column, operation: @escaping @Sendable () async -> Void) -> Cancellable { _registerSignalHandler(match: .removalOnly, once: true, cancelPrevious: false, name: name, function: function, priority: priority, fileAndLine: FileAndLine(fileID: fileID, filePath: filePath, line: line, column: column)) { _ in @@ -51,14 +105,28 @@ public extension ModelNode { } } - /// Runs every handler registered for `key` on the models `relation` reaches, - /// concurrently, and returns when all of those runs have finished. Cancelling the - /// calling task cancels those runs. + /// Runs every handler registered for `key` on the models `relation` reaches — all + /// concurrently — and returns when all of those runs have finished. + /// + /// Reach works like `send(_:to:)`: by default this model and its + /// descendants; pass `.ancestors` to ask the models above, add `.dependencies` to + /// include dependency models. Signals are repeatable. Cancelling the calling task + /// cancels the runs this call started, so a deadline around a signal reaches the + /// handlers: + /// + /// ```swift + /// func leave() async { + /// await node.signal(Lifecycle.leave) // announce while the tree is live + /// streams = [] // then remove + /// await node.signal(Lifecycle.flush) + /// } + /// ``` func signal(_ key: some Hashable & Sendable, to relation: ModelRelation = [.self, .descendants]) async { await _signal(CancellableKey(key: key), to: relation) } - /// Runs every signal handler (any key) on the models `relation` reaches. + /// Runs every signal handler, whatever its key, on the models `relation` reaches, + /// and returns when all of those runs have finished. See ``signal(_:to:)``. func signal(to relation: ModelRelation = [.self, .descendants]) async { await _signal(nil, to: relation) } @@ -67,8 +135,8 @@ public extension ModelNode { private extension ModelNode { func _registerSignalHandler(match: SignalHandler.Match, once: Bool, cancelPrevious: Bool, name: String?, function: StaticString, priority: TaskPriority?, fileAndLine: FileAndLine, perform: @escaping @Sendable (SignalCause) async -> Void) -> Cancellable { guard let context = enforcedContext() else { return EmptyCancellable() } - // Resolved NOW, while the model is live: after removal the context is sealed - // and its root may be gone. + // Resolved NOW, while the model is live: by the final `.removed` call the context + // is sealed and its root may be gone. let access = context.rootParent.modelAccess return SignalHandler( cancellations: context.cancellations, @@ -76,7 +144,7 @@ private extension ModelNode { modelName: typeDescription, taskName: name ?? "\(function) @ \(fileAndLine.description)", fileAndLine: fileAndLine, - host: access?.teardownWorkStore, access: access, + host: access?.signalWorkStore, access: access, dependencies: context.capturedDependencies, executor: _TestExecutorBox.current, priority: priority, operation: perform @@ -91,7 +159,7 @@ private extension ModelNode { } let runs = handlers.compactMap { $0.start(.requested) } // Structured like a task group: cancelling the caller cancels the runs this call - // started (e.g. a deadline wrapped around the signal). + // started. await withTaskCancellationHandler { for run in runs { _ = try? await run.value @@ -101,133 +169,3 @@ private extension ModelNode { } } } - -final class SignalHandler: Cancellable, InternalCancellable, @unchecked Sendable { - enum Match { case any, key(CancellableKey), removalOnly } - - let id: Int - weak var cancellations: Cancellations? - let match: Match - let once: Bool - let cancelPrevious: Bool - let modelName: String - let taskName: String - let fileAndLine: FileAndLine - let host: Cancellations? - let access: ModelAccess? - let dependencies: DependencyValues - let executor: (any Sendable)? - let priority: TaskPriority? - let operation: @Sendable (SignalCause) async -> Void - - private let lock = NSLock() - private var hasRun = false - private var isUnregistered = false - private var tail: Task? - - init(cancellations: Cancellations, match: Match, once: Bool, cancelPrevious: Bool, modelName: String, taskName: String, fileAndLine: FileAndLine, host: Cancellations?, access: ModelAccess?, dependencies: DependencyValues, executor: (any Sendable)?, priority: TaskPriority?, operation: @escaping @Sendable (SignalCause) async -> Void) { - self.cancellations = cancellations - self.id = cancellations.nextId - self.match = match - self.once = once - self.cancelPrevious = cancelPrevious - self.modelName = modelName - self.taskName = taskName - self.fileAndLine = fileAndLine - self.host = host - self.access = access - self.dependencies = dependencies - self.executor = executor - self.priority = priority - self.operation = operation - cancellations.register(self) - } - - func matches(_ key: CancellableKey?) -> Bool { - switch match { - case .removalOnly: return false - case .any: return true - case .key(let own): return key == nil || key == own - } - } - - /// From the model's store. A sealed store is a removal → the final call. Otherwise - /// it is a user cancellation (`cancelAll(for:)`) → unregister only. - func onCancel() { - if cancellations?.isSealed ?? true { - // The harness's own end-of-test teardown defers it past the exhaustion check. - if access?.deferRemovalCall({ [self] in _ = self.start(.removed) }) == true { return } - _ = start(.removed) - } else { - lock { isUnregistered = true } - } - } - - func cancel() { - lock { isUnregistered = true } - _ = cancellations?.unregister(id) - } - - @discardableResult - func cancel(for key: some Hashable & Sendable, cancelInFlight: Bool) -> Self { - cancellations?.cancel(self, for: key, cancelInFlight: cancelInFlight) - return self - } - - /// Starts one run (serialized behind, or cancelling, the previous one). `nil` when - /// the handler is unregistered or `once` and already run. - func start(_ cause: SignalCause) -> Task? { - let operation = self.operation - let dependencies = self.dependencies - let access = self.access - let executor = self.executor - let priority = self.priority - let taskName = self.taskName - let started = LockIsolated(false) - - // Read the previous run AND publish this one in the same critical section: - // two concurrent `signal`s must see each other, or both run unserialized. - // Spawning inside the lock is safe — neither `Task.init` nor the host store's - // `register` (never sealed) calls back into this handler. - let (task, previous): (Task?, Task?) = lock { - if isUnregistered { return (nil, nil) } - if once && hasRun { return (nil, nil) } - hasRun = true - let previous = tail - // Always wait for the previous run — with `cancelPrevious` it is cancelled - // first, but runs of one handler never overlap (as `forEach(cancelPrevious:)` - // starts the next body only after the previous one has fully unwound). - let serialized = previous - - let makeTask: @Sendable (@escaping @Sendable () -> Void) -> Task = { onDone in - let body = { @Sendable () async throws -> Void in - defer { onDone() } - _ = try? await serialized?.value - await DependencyValues.$_current.withValue(dependencies) { - started.setValue(true) - access?.taskBodyStarted() - await operation(cause) - } - } - #if canImport(Dispatch) - if #available(macOS 15.0, iOS 18.0, tvOS 18.0, watchOS 11.0, *), - let exec = executor as? _DrainTestExecutor { - return Task(executorPreference: exec, priority: priority, operation: body) - } - #endif - return Task(name: taskName, priority: priority, operation: body) - } - - let task: Task? - if let host { - task = TaskCancellable(modelName: modelName, taskName: taskName, fileAndLine: fileAndLine, cancellations: host, hasStartedRunningBox: started, task: makeTask).underlyingTask - } else { - task = makeTask {} - } - tail = task - return (task, previous) - } - if cancelPrevious { previous?.cancel() } - return task - } -} diff --git a/Sources/SwiftModel/Testing/ModelTestingSupport.swift b/Sources/SwiftModel/Testing/ModelTestingSupport.swift index a015a346..3358ffca 100644 --- a/Sources/SwiftModel/Testing/ModelTestingSupport.swift +++ b/Sources/SwiftModel/Testing/ModelTestingSupport.swift @@ -323,8 +323,8 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch // The seal makes that drain unnecessary. tester.access.context.sealRecursively() - // SPIKE: from here on removals are the harness's, not the test's — their removal - // calls are deferred past the exhaustion check (see `TestAccess.deferredRemovals`). + // From here on removals are the harness's, not the test's: their signal-handler + // removal calls are deferred past the exhaustion check (`TestAccess.deferredRemovals`). tester.access.beginHarnessTeardown() // Phase 2: Cancel all currently-registered onActivate tasks. @@ -339,7 +339,7 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch tester.access.checkExhaustion(at: fileAndLine, includeUpdates: false, checkTasks: true) tester.access.context.onRemoval() - // SPIKE: now run the harness teardown's removal calls — unchecked — until quiet, + // Now run the harness teardown's removal calls — unchecked — until quiet, // so "let the model go at scope exit, then assert its cleanup ran" works; cancel // whatever is still parked (e.g. on a clock nobody advances). let deferred = tester.access.takeDeferredRemovals() @@ -347,16 +347,16 @@ package final class _ConcreteModelTestScope: _AnyModelTestScope, @unch if !deferred.isEmpty { _ = await tester.access.waitUntilSettled(cleanup: true, at: fileAndLine) } - await tester.access.cancelTeardownWorkAndAwaitUnwind(at: fileAndLine) + await tester.access.cancelSignalWorkAndAwaitUnwind(at: fileAndLine) } package func cancelAndCleanup() { // Mark tester so its deinit skips cleanup — we are running it here instead. tester.cleanupHandledExternally = true - tester.access.beginHarnessTeardown() // SPIKE: cancelled test — deferred removal calls are dropped + tester.access.beginHarnessTeardown() // cancelled test: deferred removal calls are dropped tester.access.context.cancelAllRecursively(for: ContextCancellationKey.onActivate) tester.access.context.onRemoval() - tester.access.teardownWork.cancelAll() + tester.access.signalWork.cancelAll() } package func waitForTeardown() async { diff --git a/Tests/SwiftModelTests/SignalTests.swift b/Tests/SwiftModelTests/SignalTests.swift index d6ea0516..30b645b7 100644 --- a/Tests/SwiftModelTests/SignalTests.swift +++ b/Tests/SwiftModelTests/SignalTests.swift @@ -5,7 +5,7 @@ import Clocks import Foundation import IssueReporting -// SPIKE acceptance tests for `onSignal` / `signal`. +// Tests for `onSignal` / `signal`. private enum Lifecycle: Hashable, Sendable { case flush, leave } diff --git a/Tests/SwiftModelTests/TeardownWorkTests.swift b/Tests/SwiftModelTests/TeardownWorkTests.swift index 075ba72a..57816bbf 100644 --- a/Tests/SwiftModelTests/TeardownWorkTests.swift +++ b/Tests/SwiftModelTests/TeardownWorkTests.swift @@ -5,7 +5,7 @@ import Clocks import Foundation import IssueReporting -// SPIKE acceptance tests for `node.onTeardown` — async work that starts when a model +// Tests for `node.onTeardown` — async work that starts when a model // is deactivated and outlives it. The motivating case is an audio fade that must keep // running after its player's model is removed (a stream switch), stepping on an // injected clock. Started as a raw `Task` from `onCancel`, such work is invisible to