From d3fd66ff2ea086e294fe6913c0bfec0036b215eb Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Thu, 22 Oct 2026 23:51:10 -0400 Subject: [PATCH] Refactor: one endMoving() for the move and the delete from the Device (#890) moveDriveOnlyTakes and releaseFromDevice each had the same block that clears isMoving and resumes the edits that waited for a move. It is now endMoving(), and releaseFromDevice lets the Drive go with a defer (once, on every way out). One difference: the Drive stays held through the few lines that update the moves list and read the Drive's Take.json, where it was let go before them. Assisted-by: Claude Code:claude-sonnet-5-5 --- .../Sources/Recording/Recorder.swift | 28 ++++++++--------- .../RecordingTests/RecorderReleaseTests.swift | 31 +++++++++++++++++++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift b/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift index 1e8805d9..6fd8fc99 100644 --- a/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift +++ b/Packages/ShowRecorderKit/Sources/Recording/Recorder.swift @@ -1531,11 +1531,8 @@ public final class Recorder { guard !isRecording, !isStarting, !isFinishing, !isMoving, let access = driveFolder() else { return [] } isMoving = true defer { - isMoving = false foundTakesToMove = false - let waiting = moveWaiters - moveWaiters = [] - for waiter in waiting { waiter.resume() } + endMoving() } // A Show detail rename already running finishes first; one asked for from now on waits for this move. while editsInProgress > 0 { @@ -1683,26 +1680,19 @@ public final class Recorder { if let refusal = releaseRefusal(show: show, take: take) { return .kept([refusal]) } guard let access = driveFolder() else { return .kept(["The Drive isn't connected."]) } isMoving = true - defer { - isMoving = false - let waiting = moveWaiters - moveWaiters = [] - for waiter in waiting { waiter.resume() } - } + // The order is load-bearing: the access ends first, then `endMoving` lets the edits that waited go on. + defer { endMoving() } + defer { access.end() } // A Show detail edit already running finishes first, as for a move. while editsInProgress > 0 { await withCheckedContinuation { editWaiters.append($0) } } - if let refusal = releaseRefusal(show: show, take: take) { - access.end() - return .kept([refusal]) - } + if let refusal = releaseRefusal(show: show, take: take) { return .kept([refusal]) } let name = TakeMetadata.name(take: take) let deviceTake = deviceFolder.appending(path: show).appending(path: name) let driveTake = access.folder.appending(path: show).appending(path: name) let shouldStop = moveStop.latch() let outcome = await runOffPool { DeviceTakeRelease.run(deviceTake: deviceTake, driveTake: driveTake, shouldStop: shouldStop) } - access.end() guard outcome == .released else { return outcome } // It is no longer kept on this device: off the moves list, and the last Take reads as moved if it is that one. lastMoveRun.moves.removeAll { $0.show == show && $0.take == take } @@ -1779,6 +1769,14 @@ public final class Recorder { /// Edits waiting for a move to end (#630). @ObservationIgnored private var moveWaiters: [CheckedContinuation] = [] + /// Ends a move or a delete from the Device: `isMoving` is false and every edit that waited for it gets going (#630). + private func endMoving() { + isMoving = false + let waiting = moveWaiters + moveWaiters = [] + for waiter in waiting { waiter.resume() } + } + /// Returns once no move is running (#630). On the main actor, what the caller does next runs before any move can /// start. func waitWhileMoving() async { diff --git a/Packages/ShowRecorderKit/Tests/RecordingTests/RecorderReleaseTests.swift b/Packages/ShowRecorderKit/Tests/RecordingTests/RecorderReleaseTests.swift index a16d9a3e..715f41a9 100644 --- a/Packages/ShowRecorderKit/Tests/RecordingTests/RecorderReleaseTests.swift +++ b/Packages/ShowRecorderKit/Tests/RecordingTests/RecorderReleaseTests.swift @@ -74,6 +74,37 @@ struct RecorderReleaseTests { #expect(recorder.moveReport.entries.isEmpty, "no longer listed as kept on the iPhone") } + final class Ended: @unchecked Sendable { + private let lock = NSLock() + private var value = 0 + func bump() { lock.withLock { value += 1 } } + var count: Int { lock.withLock { value } } + } + + @Test("Each delete lets the Drive go exactly once, whether it deletes or keeps the Take, and leaves no move running (#890)") + func driveLetGoOnce() async throws { + defer { try? FileManager.default.removeItem(at: root) } + try makeKeptTake() + let ended = Ended() + let drive = drive + let recorder = Recorder( + deviceFolder: device, driveFolder: { DriveAccess(folder: drive, stop: { ended.bump() }) }, + now: { RecordingTakeTests.showDay }, destinationCheckInterval: nil) + + // The Drive lacks a Stem: kept. + let hidden = driveTake.appending(path: DeviceTakeReleaseTests.channel(2).stemFile) + let aside = root.appending(path: "aside.wav") + try FileManager.default.moveItem(at: hidden, to: aside) + guard case .kept = await recorder.releaseFromDevice(show: "2026-10-06 Show", take: 1) else { Issue.record("not kept"); return } + #expect(ended.count == 1, "kept") + #expect(!recorder.isMoving) + + try FileManager.default.moveItem(at: aside, to: hidden) + #expect(await recorder.releaseFromDevice(show: "2026-10-06 Show", take: 1) == .released) + #expect(ended.count == 2, "released") + #expect(!recorder.isMoving) + } + @Test("Not during a Take, and not during a save: the Take stays") func neverWhileABusyTake() async throws { defer { try? FileManager.default.removeItem(at: root) }