Skip to content

Commit 2eb5d3f

Browse files
macOS updater: download + swap off the main actor
The in-app update download reliably died inside the app: the byte loop ran on the main actor, so every element hopped through the main executor and the transfer starved (observed live: -1001 request timeouts at 25 %, or all bytes consumed and the loop suspended forever waiting for a stream EOF that HTTP/2 never delivered — while the byte-identical loop completes in seconds off the main actor; verified with a standalone harness against the same URL). downloadAndInstall now delegates to a nonisolated static that owns the whole download → digest → unpack → verify → swap sequence and returns to the main actor only for the final terminate. Two hardening fixes ride along: - consume AsyncBytes through a 64 KiB buffer instead of hashing and appending per byte (31M CryptoKit calls), and - stop at the size the signed manifest pins instead of waiting for the stream's own EOF. Progress hops to the main actor in AppModel before touching observable state.
1 parent 25e6055 commit 2eb5d3f

2 files changed

Lines changed: 62 additions & 20 deletions

File tree

‎macos-host/Sources/RivetHost/AppModel.swift‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,13 @@ final class AppModel {
319319
guard let self else { return }
320320
do {
321321
try await updateService.downloadAndInstall(manifest) { [weak self] percent in
322-
self?.updateProgressPercent = percent
322+
// The download runs off the main actor; hop the
323+
// observable write back so SwiftUI sees it.
324+
// (_Concurrency-prefixed: the generated Task record
325+
// shadows the concurrency Task in this module.)
326+
_Concurrency.Task { @MainActor [weak self] in
327+
self?.updateProgressPercent = percent
328+
}
323329
}
324330
} catch {
325331
updateErrorMessage = t("updateInstallFailed", Self.cleanError(error))

‎macos-host/Sources/RivetHost/UpdateService.swift‎

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ public final class UpdateService {
6060
let signature: SignatureBlock
6161
}
6262

63-
public struct Manifest: Decodable {
64-
public struct Artifact: Decodable {
63+
public struct Manifest: Decodable, Sendable {
64+
public struct Artifact: Decodable, Sendable {
6565
let platform: String
6666
let architecture: String
6767
let url: URL
@@ -93,7 +93,7 @@ public final class UpdateService {
9393
keyOverride: String? = nil) {
9494
self.i18n = i18n
9595
let config = sessionConfiguration ?? URLSessionConfiguration.ephemeral
96-
config.timeoutIntervalForRequest = 30
96+
config.timeoutIntervalForRequest = 60
9797
config.timeoutIntervalForResource = 600
9898
self.session = URLSession(configuration: config)
9999
self.feedURL = feedURL ?? Self.manifestURL
@@ -201,19 +201,41 @@ public final class UpdateService {
201201
/// install when anything is off. `onProgress` reports download percent
202202
/// (0…100) on the main actor for the update sheet.
203203
public func downloadAndInstall(_ manifest: Manifest,
204-
onProgress: ((Int) -> Void)? = nil) async throws {
204+
onProgress: (@Sendable (Int) -> Void)? = nil) async throws {
205205
guard let bundleURL = installedBundleURL else { throw UpdateError.notInstalled }
206206
guard let artifact = manifest.artifact(forPlatform: "macos") else {
207207
throw UpdateError.manifestMissing
208208
}
209+
try await Self.downloadAndSwap(artifact: artifact,
210+
bundleURL: bundleURL,
211+
expectedVersion: manifest.version,
212+
onProgress: onProgress ?? { _ in })
213+
}
209214

215+
/// The whole download → verify → swap sequence, deliberately off the
216+
/// main actor: consuming AsyncBytes on the main actor starves the
217+
/// transfer (every element hops through the main executor, the
218+
/// connection stalls and URLSession times out — observed live against
219+
/// the GitHub CDN: the identical loop completes in seconds off the
220+
/// main actor and dies partway through on it).
221+
nonisolated private static func downloadAndSwap(
222+
artifact: Manifest.Artifact,
223+
bundleURL: URL,
224+
expectedVersion: String,
225+
onProgress: @Sendable (Int) -> Void
226+
) async throws {
210227
// Download to a temp directory.
211228
let workDir = FileManager.default.temporaryDirectory
212229
.appendingPathComponent("taskly-update-\(UUID().uuidString)")
213230
try FileManager.default.createDirectory(at: workDir, withIntermediateDirectories: true)
214231
let zipURL = workDir.appendingPathComponent("taskly-update.zip")
215232
defer { try? FileManager.default.removeItem(at: workDir) }
216233

234+
let config = URLSessionConfiguration.ephemeral
235+
config.timeoutIntervalForRequest = 60
236+
config.timeoutIntervalForResource = 600
237+
let session = URLSession(configuration: config)
238+
217239
do {
218240
let (bytes, response) = try await session.bytes(from: artifact.url)
219241
guard let http = response as? HTTPURLResponse, http.statusCode == 200 else {
@@ -224,17 +246,33 @@ public final class UpdateService {
224246
data.reserveCapacity(artifact.size)
225247
let total = max(artifact.size, 1)
226248
var lastPercent = -1
249+
// Consume AsyncBytes through a 64 KiB buffer: hashing/appending
250+
// per byte (31M CryptoKit calls) is far slower than the wire.
251+
var buffer = [UInt8]()
252+
buffer.reserveCapacity(65_536)
253+
func absorb(_ chunk: [UInt8]) {
254+
hasher.update(data: chunk)
255+
data.append(contentsOf: chunk)
256+
let percent = min(100, data.count * 100 / total)
257+
if percent != lastPercent {
258+
lastPercent = percent
259+
onProgress(percent)
260+
}
261+
}
227262
for try await byte in bytes {
228-
hasher.update(data: [byte])
229-
data.append(byte)
230-
if onProgress != nil {
231-
let percent = min(100, data.count * 100 / total)
232-
if percent != lastPercent {
233-
lastPercent = percent
234-
onProgress?(percent)
235-
}
263+
buffer.append(byte)
264+
if buffer.count == buffer.capacity
265+
|| data.count + buffer.count >= total {
266+
absorb(buffer)
267+
buffer.removeAll(keepingCapacity: true)
236268
}
269+
// The signed manifest pins the exact size — stop as soon as
270+
// we have it. Waiting for the stream's own EOF can hang
271+
// forever on responses whose end-of-stream flag never
272+
// surfaces to AsyncBytes.
273+
if data.count >= total { break }
237274
}
275+
if !buffer.isEmpty { absorb(buffer) }
238276
let digest = hasher.finalize().map { String(format: "%02x", $0) }.joined()
239277
guard digest == artifact.sha256.lowercased() else {
240278
throw UpdateError.checksumMismatch
@@ -257,15 +295,13 @@ public final class UpdateService {
257295
guard ditto.terminationStatus == 0 else { throw UpdateError.checksumMismatch }
258296

259297
let newBundle = unpacked.appendingPathComponent("Taskly.app")
260-
let newInfoPlist = newBundle.appendingPathComponent("Contents/Info.plist")
261298
guard let newVersion = Bundle(url: newBundle)?
262299
.infoDictionary?["CFBundleShortVersionString"] as? String else {
263-
throw UpdateError.versionMismatch(expected: manifest.version, got: "missing")
300+
throw UpdateError.versionMismatch(expected: expectedVersion, got: "missing")
264301
}
265-
guard newVersion == manifest.version else {
266-
throw UpdateError.versionMismatch(expected: manifest.version, got: newVersion)
302+
guard newVersion == expectedVersion else {
303+
throw UpdateError.versionMismatch(expected: expectedVersion, got: newVersion)
267304
}
268-
_ = newInfoPlist
269305

270306
// Swap: the shell script survives our exit; it also restores the
271307
// old bundle if the new one fails to move in. ~/.taskly is never
@@ -300,10 +336,10 @@ public final class UpdateService {
300336
bash.executableURL = URL(fileURLWithPath: "/bin/bash")
301337
bash.arguments = [swapPath.path]
302338
try bash.run()
303-
NSApplication.shared.terminate(nil)
339+
await MainActor.run { NSApplication.shared.terminate(nil) }
304340
}
305341

306-
private static func q(_ path: String) -> String {
342+
nonisolated private static func q(_ path: String) -> String {
307343
"'" + path.replacingOccurrences(of: "'", with: "'\\''") + "'"
308344
}
309345

0 commit comments

Comments
 (0)