From bd11ba96c5f63c717957244950ca3273fc632571 Mon Sep 17 00:00:00 2001 From: kua-agent Date: Sat, 5 Sep 2026 09:27:49 +0400 Subject: [PATCH] =?UTF-8?q?fix:=20MAJOR=20=E2=80=94=20send()=20race=20betw?= =?UTF-8?q?een=20concurrent=20transport=20switch=20and=20an=20in-flight=20?= =?UTF-8?q?send?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues: (1) directoryExists(at:) only checked existence + isDirectory, so an existing-but-unwritable OneDrive folder skipped the oneDriveFolderUnavailable branch and surfaced as a generic pdfCompositionFailed message instead — now uses OneDriveLocator.isWritableDirectory(at:). (2) send() read `self.outboxURL` again inside composePDFForSend() after at least one await had already run, so a concurrent chooseTransport() call (SettingsView.swift) could flip transport/outboxURL/recordUncommented mid-send, landing the PDF under one transport's folder while the archive/status branch ran the other's. Fix: send() now snapshots BOTH transport and the destination folder into local `let`s once, before any await, and passes the folder explicitly into the renamed composePDFForSend(outbox:) — which no longer reads self.outboxURL at all. The archive/status switch already used the frozen `transport` local. (chooseTransport/chooseOneDriveFolder additionally refuse outright while isSending is true — see the SettingsView.swift commit — so in practice this race can no longer even be triggered, but the snapshot is the actual structural fix regardless.) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp --- Sources/Shotdeck/SendController.swift | 45 ++++++++++++++++----------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/Sources/Shotdeck/SendController.swift b/Sources/Shotdeck/SendController.swift index 6849703..99f0602 100644 --- a/Sources/Shotdeck/SendController.swift +++ b/Sources/Shotdeck/SendController.swift @@ -16,17 +16,28 @@ extension AppModel: SendCapable { guard !session.isEmpty, !isSending else { return } setSending(true) + // Snapshot BOTH the transport AND the destination folder into local `let`s + // ONCE, before any `await` in this function. chooseTransport/chooseOneDriveFolder + // now refuse (status "Finish the current send first.") while isSending is true, + // but this snapshot is the actual fix for the race: even without that guard, + // everything below operates on these frozen values — composePDFForSend(outbox:) + // takes the folder as a parameter and never re-reads `self.outboxURL` after a + // suspension point, so a concurrent transport switch mid-send can no longer land + // the PDF under one transport's folder while the archive/status branch (which + // switches on the same frozen `transport` local) runs the other's. let transport = TransportSettings.transport() + let destinationFolder: URL - // OneDrive mode: verify the real destination exists RIGHT NOW, before composing - // anything. `outboxURL` is kept in sync with the resolved OneDrive folder by - // bootstrap/chooseTransport/chooseOneDriveFolder, but this is re-resolved fresh - // here (never trusted stale) so a folder that vanished since then (OneDrive - // signed out, external volume unmounted, folder deleted) is caught instead of - // silently writing into whatever `outboxURL` happens to hold. + // OneDrive mode: verify the real destination exists AND is writable RIGHT NOW, + // before composing anything. `outboxURL` is kept in sync with the resolved + // OneDrive folder by bootstrap/chooseTransport/chooseOneDriveFolder, but this is + // re-resolved fresh here (never trusted stale) so a folder that vanished or lost + // its permissions since then (OneDrive signed out, external volume unmounted, + // folder deleted, chmod'd unwritable) is caught instead of silently attempted + // and surfacing as a generic PDF-composition failure. if transport == .oneDrive { guard let folder = OneDriveLocator.resolveOneDriveFolder(), - Self.directoryExists(at: folder) + OneDriveLocator.isWritableDirectory(at: folder) else { let path = OneDriveLocator.resolveOneDriveFolder()?.path ?? TransportSettings.storedOneDriveFolderPath() @@ -36,16 +47,19 @@ extension AppModel: SendCapable { setSending(false) return } + destinationFolder = folder setResolvedOneDriveFolder(folder) if outboxURL != folder || watchFolderURL != folder { setFolderURLs(outbox: folder, watch: folder) try? await watcher.updateWatchFolder(folder) } + } else { + destinationFolder = outboxURL } let pending: ComposedSend do { - pending = try await composePDFForSend() + pending = try await composePDFForSend(outbox: destinationFolder) } catch { // Never unlink the published PDF, and never unlink the temp file either: // a rename failure would leave the complete document at the temp name. @@ -93,19 +107,14 @@ extension AppModel: SendCapable { } } - private static func directoryExists(at url: URL) -> Bool { - var isDirectory: ObjCBool = false - let exists = FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory) - return exists && isDirectory.boolValue - } - - /// Writes the PDF to the outbox and records its path. Does not archive the session + /// Writes the PDF to `outboxDir` and records its path. Does not archive the session /// and does not present AirDrop — that happens only after the share completes. - func composePDFForSend() async throws -> ComposedSend { + /// `outboxDir` is passed in (a value `send(anchor:)` snapshotted before any await) + /// rather than read from `self.outboxURL` here, so a concurrent transport switch + /// mid-send can never redirect an in-flight compose to a different folder. + func composePDFForSend(outbox outboxDir: URL) async throws -> ComposedSend { let workingSession = session let composer = self.composer - // Live outbox (FolderSettings), not `paths.outbox` — Settings changes take effect. - let outboxDir = outboxURL let sourceDir = paths.sessionDirectory(workingSession.id) let fileName = PDFComposer.fileName(for: workingSession) let finalURL = outboxDir.appendingPathComponent(fileName)