fix: prevent rendering and SSH teardown crashes
This commit is contained in:
1 parent
8f037726ec
commit
b3d3de5913
8 files changed
+348
-79
No files matched your search
@@ -0,0 +1,172 @@
|
||||
import Foundation
|
||||
import Observation
|
||||
import TermindApplication
|
||||
import TermindDomain
|
||||
import Testing
|
||||
|
||||
@Suite @MainActor
|
||||
struct HistoryRenderingSnapshotTests {
|
||||
@Test func failedInitialTrimDoesNotHideReadableHistory() throws {
|
||||
let repository = HistoryRepository()
|
||||
let entry = ConnectionHistoryEntry(hostID: UUID(), kind: .ssh)
|
||||
try repository.insert(entry, limit: 10)
|
||||
repository.failsTrim = true
|
||||
|
||||
let model = ConnectionHistoryModel(repository: repository)
|
||||
|
||||
#expect(model.entries().map(\.id) == [entry.id])
|
||||
#expect(model.storageError != nil)
|
||||
model.reload()
|
||||
#expect(model.entries().map(\.id) == [entry.id])
|
||||
#expect(model.storageError == nil)
|
||||
}
|
||||
|
||||
@Test func failedInitialRegistrationDoesNotHideReadableDeviceNames() throws {
|
||||
let repository = DeviceRepository()
|
||||
let remote = DeviceRecord(id: UUID(), displayName: "Remote")
|
||||
try repository.upsert(remote)
|
||||
repository.failsWrites = true
|
||||
|
||||
let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local")
|
||||
|
||||
#expect(model.name(for: remote.id) == "Remote")
|
||||
#expect(model.name(for: model.currentDeviceID) == "Local")
|
||||
#expect(model.storageError != nil)
|
||||
model.reload()
|
||||
#expect(model.name(for: remote.id) == "Remote")
|
||||
#expect(model.storageError == nil)
|
||||
}
|
||||
|
||||
@Test(arguments: [false, true])
|
||||
func historyReadsDoNotFetchOrPublishDuringRendering(fails: Bool) {
|
||||
let repository = HistoryRepository()
|
||||
repository.fails = fails
|
||||
let model = ConnectionHistoryModel(repository: repository)
|
||||
let hostID = UUID()
|
||||
let reads = repository.reads
|
||||
withObservationTracking {
|
||||
_ = model.storageError
|
||||
_ = model.entries()
|
||||
} onChange: {
|
||||
Issue.record("Reading history must not publish observable changes")
|
||||
}
|
||||
|
||||
for _ in 0..<2 {
|
||||
_ = model.entries()
|
||||
_ = model.entries(for: hostID)
|
||||
_ = model.mostRecentDate(for: hostID)
|
||||
_ = model.localEntries()
|
||||
}
|
||||
#expect(repository.reads == reads)
|
||||
#expect((model.storageError != nil) == fails)
|
||||
}
|
||||
|
||||
@Test func historyReloadPublishesCommittedRowsAndRecoversFromReadFailure() throws {
|
||||
let repository = HistoryRepository()
|
||||
let model = ConnectionHistoryModel(repository: repository)
|
||||
let entry = ConnectionHistoryEntry(hostID: UUID(), kind: .ssh)
|
||||
try repository.insert(entry, limit: 10)
|
||||
model.reload()
|
||||
#expect(model.entries().map(\.id) == [entry.id])
|
||||
|
||||
repository.fails = true
|
||||
model.reload()
|
||||
let failedReads = repository.reads
|
||||
#expect(model.entries().isEmpty)
|
||||
#expect(model.storageError != nil)
|
||||
#expect(repository.reads == failedReads)
|
||||
|
||||
repository.fails = false
|
||||
model.reload()
|
||||
#expect(model.mostRecentDate(for: entry.hostID) == entry.connectedAt)
|
||||
#expect(model.storageError == nil)
|
||||
model.delete(entry.id)
|
||||
#expect(model.entries().isEmpty)
|
||||
}
|
||||
|
||||
@Test(arguments: [false, true])
|
||||
func deviceReadsDoNotFetchOrPublishDuringRendering(fails: Bool) {
|
||||
let repository = DeviceRepository()
|
||||
repository.fails = fails
|
||||
let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local")
|
||||
let reads = repository.reads
|
||||
withObservationTracking {
|
||||
_ = model.storageError
|
||||
_ = model.devices()
|
||||
} onChange: {
|
||||
Issue.record("Reading device names must not publish observable changes")
|
||||
}
|
||||
|
||||
for _ in 0..<2 {
|
||||
_ = model.devices()
|
||||
#expect(model.name(for: model.currentDeviceID) == "Local")
|
||||
#expect(model.name(for: UUID()) == nil)
|
||||
}
|
||||
#expect(repository.reads == reads)
|
||||
#expect((model.storageError != nil) == fails)
|
||||
}
|
||||
|
||||
@Test func deviceReloadPublishesCommittedNamesAndRecoversFromReadFailure() throws {
|
||||
let repository = DeviceRepository()
|
||||
let model = DeviceDirectoryModel(repository: repository, currentDeviceName: "Local")
|
||||
let remote = DeviceRecord(id: UUID(), displayName: "Remote")
|
||||
try repository.upsert(remote)
|
||||
model.reload()
|
||||
#expect(model.name(for: remote.id) == "Remote")
|
||||
|
||||
repository.fails = true
|
||||
model.reload()
|
||||
let failedReads = repository.reads
|
||||
#expect(model.devices().isEmpty)
|
||||
#expect(model.storageError != nil)
|
||||
#expect(repository.reads == failedReads)
|
||||
|
||||
repository.fails = false
|
||||
model.reload()
|
||||
#expect(model.name(for: remote.id) == "Remote")
|
||||
#expect(model.storageError == nil)
|
||||
model.updateCurrentDeviceName("Renamed")
|
||||
#expect(model.name(for: model.currentDeviceID) == "Renamed")
|
||||
}
|
||||
|
||||
private enum ReadFailure: Error { case unavailable }
|
||||
|
||||
private final class HistoryRepository: ConnectionHistoryRepository {
|
||||
let storage = InMemoryConnectionHistoryRepository()
|
||||
var reads = 0
|
||||
var fails = false
|
||||
var failsTrim = false
|
||||
|
||||
func entries() throws -> [ConnectionHistoryEntry] {
|
||||
reads += 1
|
||||
if fails { throw ReadFailure.unavailable }
|
||||
return try storage.entries()
|
||||
}
|
||||
func trim(to limit: Int) throws {
|
||||
if failsTrim { throw ReadFailure.unavailable }
|
||||
try storage.trim(to: limit)
|
||||
}
|
||||
func insert(_ entry: ConnectionHistoryEntry, limit: Int) throws { try storage.insert(entry, limit: limit) }
|
||||
func end(_ id: UUID, at date: Date) throws { try storage.end(id, at: date) }
|
||||
func delete(_ id: UUID) throws { try storage.delete(id) }
|
||||
func clear(for hostID: UUID) throws { try storage.clear(for: hostID) }
|
||||
func clearAll() throws { try storage.clearAll() }
|
||||
}
|
||||
|
||||
private final class DeviceRepository: DeviceDirectoryRepository {
|
||||
let storage = InMemoryDeviceDirectoryRepository()
|
||||
var reads = 0
|
||||
var fails = false
|
||||
var failsWrites = false
|
||||
|
||||
func devices() throws -> [DeviceRecord] {
|
||||
reads += 1
|
||||
if fails { throw ReadFailure.unavailable }
|
||||
return try storage.devices()
|
||||
}
|
||||
func upsert(_ device: DeviceRecord) throws {
|
||||
if failsWrites { throw ReadFailure.unavailable }
|
||||
try storage.upsert(device)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
import Dispatch
|
||||
import Logging
|
||||
import Testing
|
||||
|
||||
@testable import TermindRemoteInfrastructure
|
||||
|
||||
@Suite
|
||||
struct SSHConnectionWorkerLifecycleTests {
|
||||
@Test func lastReferenceCanBeReleasedOnTheConnectionQueue() async {
|
||||
let queue = DispatchQueue(label: "termind.tests.ssh.connection")
|
||||
await withCheckedContinuation { continuation in
|
||||
queue.async {
|
||||
var worker: SSHConnectionWorker? = SSHConnectionWorker(
|
||||
logger: Logger(label: "termind.tests"), maxSessionsPolicy: SSHMaxSessionsPolicy(), queue: queue)
|
||||
weak let released = worker
|
||||
#expect(released != nil)
|
||||
worker = nil
|
||||
#expect(released == nil)
|
||||
continuation.resume()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test func lastReferenceCanBeReleasedOutsideTheConnectionQueue() {
|
||||
let queue = DispatchQueue(label: "termind.tests.ssh.connection")
|
||||
dispatchPrecondition(condition: .notOnQueue(queue))
|
||||
var worker: SSHConnectionWorker? = SSHConnectionWorker(
|
||||
logger: Logger(label: "termind.tests"), maxSessionsPolicy: SSHMaxSessionsPolicy(), queue: queue)
|
||||
weak let released = worker
|
||||
#expect(released != nil)
|
||||
worker = nil
|
||||
#expect(released == nil)
|
||||
}
|
||||
}
|
||||
@@ -18,7 +18,7 @@ public final class ConnectionHistoryModel {
|
||||
|
||||
private let repository: any ConnectionHistoryRepository
|
||||
public let sourceDeviceID: UUID
|
||||
private var cachedEntries: [ConnectionHistoryEntry]?
|
||||
private var cachedEntries: [ConnectionHistoryEntry] = []
|
||||
public private(set) var revision: UInt64 = 0
|
||||
public private(set) var storageError: String?
|
||||
|
||||
@@ -34,20 +34,17 @@ public final class ConnectionHistoryModel {
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
// A failed maintenance write must not hide readable existing history.
|
||||
do {
|
||||
cachedEntries = try self.repository.entries()
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
}
|
||||
|
||||
public func entries() -> [ConnectionHistoryEntry] {
|
||||
_ = revision
|
||||
if let cachedEntries { return cachedEntries }
|
||||
do {
|
||||
let values = try repository.entries()
|
||||
cachedEntries = values
|
||||
storageError = nil
|
||||
return values
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
return []
|
||||
}
|
||||
return cachedEntries
|
||||
}
|
||||
|
||||
public func entries(for hostID: Host.ID) -> [ConnectionHistoryEntry] {
|
||||
@@ -95,9 +92,7 @@ public final class ConnectionHistoryModel {
|
||||
)
|
||||
do {
|
||||
try repository.insert(entry, limit: Self.limit)
|
||||
cachedEntries = nil
|
||||
storageError = nil
|
||||
revision &+= 1
|
||||
reload()
|
||||
return entry
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
@@ -108,35 +103,34 @@ public final class ConnectionHistoryModel {
|
||||
public func end(_ id: ConnectionHistoryEntry.ID, at date: Date = Date()) {
|
||||
do {
|
||||
try repository.end(id, at: date)
|
||||
cachedEntries = nil
|
||||
storageError = nil
|
||||
revision &+= 1
|
||||
reload()
|
||||
} catch { storageError = error.localizedDescription }
|
||||
}
|
||||
|
||||
/// The sync coordinator persists remote events through its own transaction.
|
||||
/// Drop the presentation cache so the next read observes those committed rows.
|
||||
/// Refresh outside rendering, after a local write or a committed sync transaction.
|
||||
/// View reads must never publish cache or error changes into an active graph update.
|
||||
public func reload() {
|
||||
cachedEntries = nil
|
||||
storageError = nil
|
||||
do {
|
||||
cachedEntries = try repository.entries()
|
||||
storageError = nil
|
||||
} catch {
|
||||
cachedEntries = []
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
revision &+= 1
|
||||
}
|
||||
|
||||
public func delete(_ id: ConnectionHistoryEntry.ID) {
|
||||
do {
|
||||
try repository.delete(id)
|
||||
cachedEntries = nil
|
||||
storageError = nil
|
||||
revision &+= 1
|
||||
reload()
|
||||
} catch { storageError = error.localizedDescription }
|
||||
}
|
||||
|
||||
public func clear(for hostID: Host.ID) {
|
||||
do {
|
||||
try repository.clear(for: hostID)
|
||||
cachedEntries = nil
|
||||
storageError = nil
|
||||
revision &+= 1
|
||||
reload()
|
||||
} catch { storageError = error.localizedDescription }
|
||||
}
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ public final class DeviceDirectoryModel {
|
||||
private let repository: any DeviceDirectoryRepository
|
||||
public let currentDeviceID: UUID
|
||||
public private(set) var currentDeviceName: String
|
||||
private var cachedDevices: [UUID: DeviceRecord]?
|
||||
private var cachedDevices: [UUID: DeviceRecord] = [:]
|
||||
public private(set) var revision: UInt64 = 0
|
||||
public private(set) var storageError: String?
|
||||
|
||||
@@ -25,28 +25,25 @@ public final class DeviceDirectoryModel {
|
||||
|
||||
public func devices() -> [DeviceRecord] {
|
||||
_ = revision
|
||||
if let cachedDevices { return Array(cachedDevices.values) }
|
||||
do {
|
||||
let values = try repository.devices()
|
||||
cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) })
|
||||
storageError = nil
|
||||
return values
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
return []
|
||||
}
|
||||
return Array(cachedDevices.values)
|
||||
}
|
||||
|
||||
public func name(for deviceID: UUID?) -> String? {
|
||||
guard let deviceID else { return nil }
|
||||
if cachedDevices == nil { _ = devices() }
|
||||
return cachedDevices?[deviceID]?.displayName
|
||||
_ = revision
|
||||
return cachedDevices[deviceID]?.displayName
|
||||
?? (deviceID == currentDeviceID ? currentDeviceName : nil)
|
||||
}
|
||||
|
||||
public func reload() {
|
||||
cachedDevices = nil
|
||||
storageError = nil
|
||||
do {
|
||||
let values = try repository.devices()
|
||||
cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) })
|
||||
storageError = nil
|
||||
} catch {
|
||||
cachedDevices = [:]
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
revision &+= 1
|
||||
}
|
||||
|
||||
@@ -54,9 +51,7 @@ public final class DeviceDirectoryModel {
|
||||
do {
|
||||
try repository.upsert(DeviceRecord(id: currentDeviceID, displayName: name))
|
||||
currentDeviceName = name
|
||||
cachedDevices = nil
|
||||
storageError = nil
|
||||
revision &+= 1
|
||||
reload()
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
@@ -66,8 +61,13 @@ public final class DeviceDirectoryModel {
|
||||
do {
|
||||
try repository.registerIfNeeded(
|
||||
DeviceRecord(id: currentDeviceID, displayName: currentDeviceName))
|
||||
cachedDevices = nil
|
||||
storageError = nil
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
// Registration can fail while previously saved device names remain readable.
|
||||
do {
|
||||
let values = try repository.devices()
|
||||
cachedDevices = Dictionary(uniqueKeysWithValues: values.map { ($0.id, $0) })
|
||||
} catch {
|
||||
storageError = error.localizedDescription
|
||||
}
|
||||
|
||||
@@ -90,7 +90,8 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable {
|
||||
}
|
||||
}
|
||||
|
||||
private let queue = DispatchQueue(label: "com.akinokaede.termind.ssh.connection")
|
||||
private let queue: DispatchQueue
|
||||
private let queueKey = DispatchSpecificKey<Void>()
|
||||
private let logger: Logger
|
||||
private let maxSessionsPolicy: SSHMaxSessionsPolicy
|
||||
private var connection: SSHAuthenticatedConnection?
|
||||
@@ -114,9 +115,14 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable {
|
||||
private var connectionFailure: (@Sendable (String) -> Void)?
|
||||
private var didReportConnectionFailure = false
|
||||
|
||||
init(logger: Logger, maxSessionsPolicy: SSHMaxSessionsPolicy) {
|
||||
init(
|
||||
logger: Logger, maxSessionsPolicy: SSHMaxSessionsPolicy,
|
||||
queue: DispatchQueue = DispatchQueue(label: "com.akinokaede.termind.ssh.connection")
|
||||
) {
|
||||
self.queue = queue
|
||||
self.logger = logger
|
||||
self.maxSessionsPolicy = maxSessionsPolicy
|
||||
queue.setSpecific(key: queueKey, value: ())
|
||||
sftpChannelOwner = SSHMultiplexedSFTPChannelOwner(
|
||||
queue: queue,
|
||||
context: { [weak self] in
|
||||
@@ -134,7 +140,13 @@ nonisolated final class SSHConnectionWorker: @unchecked Sendable {
|
||||
}
|
||||
|
||||
deinit {
|
||||
queue.sync { cleanup(error: CancellationError()) }
|
||||
// A queued callback can release the last reference to this worker.
|
||||
// Cleanup must stay serialized without synchronously reentering its queue.
|
||||
if DispatchQueue.getSpecific(key: queueKey) != nil {
|
||||
cleanup(error: CancellationError())
|
||||
} else {
|
||||
queue.sync { cleanup(error: CancellationError()) }
|
||||
}
|
||||
}
|
||||
|
||||
func connect(
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import SwiftData
|
||||
import SwiftUI
|
||||
import TermindPresentation
|
||||
|
||||
struct MobileStartupView: View {
|
||||
let startup: Result<MobileAppModel, any Error>
|
||||
@AppStorage("mobile.appearance") private var appearance = "system"
|
||||
|
||||
nonisolated init(startup: Result<MobileAppModel, any Error>) {
|
||||
self.startup = startup
|
||||
}
|
||||
|
||||
// SwiftUI's LazyView can invoke WindowGroup content on AsyncRenderer.
|
||||
// Only carry the startup result here; access app state in the View body,
|
||||
// where SwiftUI tracks the main-actor requirement.
|
||||
nonisolated static func sceneContent(
|
||||
for startup: Result<MobileAppModel, any Error>
|
||||
) -> @Sendable () -> MobileStartupView {
|
||||
{ MobileStartupView(startup: startup) }
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
switch startup {
|
||||
case .success(let app):
|
||||
MobileQuickActionSceneRoot(app: app)
|
||||
.modelContainer(app.container)
|
||||
.preferredColorScheme(appearance == "dark" ? .dark : appearance == "light" ? .light : nil)
|
||||
case .failure:
|
||||
ContentUnavailableView(
|
||||
AppStrings.Common.unableToOpenTermind.swiftUIKey, systemImage: "exclamationmark.triangle",
|
||||
description: Text(AppStrings.Common.openDataStoreFailure))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private struct MobileQuickActionSceneRoot: View {
|
||||
let app: MobileAppModel
|
||||
@EnvironmentObject private var sceneDelegate: MobileSceneDelegate
|
||||
|
||||
var body: some View {
|
||||
MobileRootView(
|
||||
app: app,
|
||||
pendingQuickAction: sceneDelegate.pendingQuickAction,
|
||||
consumePendingQuickAction: sceneDelegate.consumePendingQuickAction
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -1,12 +1,9 @@
|
||||
import SwiftData
|
||||
import SwiftUI
|
||||
import TermindApplication
|
||||
import TermindPresentation
|
||||
|
||||
@main
|
||||
struct TermindIOSApp: App {
|
||||
@UIApplicationDelegateAdaptor(MobileAppDelegate.self) private var appDelegate
|
||||
@AppStorage("mobile.appearance") private var appearance = "system"
|
||||
private let startup: Result<MobileAppModel, any Error>
|
||||
|
||||
init() {
|
||||
@@ -18,31 +15,7 @@ struct TermindIOSApp: App {
|
||||
}
|
||||
|
||||
var body: some Scene {
|
||||
WindowGroup {
|
||||
switch startup {
|
||||
case .success(let app):
|
||||
MobileQuickActionSceneRoot(app: app)
|
||||
.modelContainer(app.container)
|
||||
.preferredColorScheme(appearance == "dark" ? .dark : appearance == "light" ? .light : nil)
|
||||
case .failure:
|
||||
ContentUnavailableView(
|
||||
AppStrings.Common.unableToOpenTermind.swiftUIKey, systemImage: "exclamationmark.triangle",
|
||||
description: Text(AppStrings.Common.openDataStoreFailure))
|
||||
}
|
||||
}
|
||||
.commands { MobileCommands() }
|
||||
}
|
||||
}
|
||||
|
||||
private struct MobileQuickActionSceneRoot: View {
|
||||
let app: MobileAppModel
|
||||
@EnvironmentObject private var sceneDelegate: MobileSceneDelegate
|
||||
|
||||
var body: some View {
|
||||
MobileRootView(
|
||||
app: app,
|
||||
pendingQuickAction: sceneDelegate.pendingQuickAction,
|
||||
consumePendingQuickAction: sceneDelegate.consumePendingQuickAction
|
||||
)
|
||||
WindowGroup(makeContent: MobileStartupView.sceneContent(for: startup))
|
||||
.commands { MobileCommands() }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,37 @@
|
||||
import Foundation
|
||||
import Testing
|
||||
|
||||
@testable import TermindIOS
|
||||
|
||||
@Suite @MainActor
|
||||
struct MobileStartupViewTests {
|
||||
private enum StartupFailure: Error { case unavailableStore }
|
||||
|
||||
@Test func successfulSceneContentCanBeCreatedOffMainThread() async throws {
|
||||
let app = try MobileAppModel(inMemory: true)
|
||||
defer { app.outbox.stop() }
|
||||
let makeContent = MobileStartupView.sceneContent(for: .success(app))
|
||||
|
||||
let content = await Task.detached {
|
||||
dispatchPrecondition(condition: .notOnQueue(.main))
|
||||
return makeContent()
|
||||
}.value
|
||||
|
||||
#expect(try content.startup.get() === app)
|
||||
}
|
||||
|
||||
@Test func failedSceneContentCanBeCreatedOffMainThread() async {
|
||||
let makeContent = MobileStartupView.sceneContent(for: .failure(StartupFailure.unavailableStore))
|
||||
|
||||
let content = await Task.detached {
|
||||
dispatchPrecondition(condition: .notOnQueue(.main))
|
||||
return makeContent()
|
||||
}.value
|
||||
|
||||
guard case .failure(let error) = content.startup else {
|
||||
Issue.record("Failed startup must remain a failure when creating scene content")
|
||||
return
|
||||
}
|
||||
#expect(error as? StartupFailure == .unavailableStore)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user