From 4c4675ef4fc4695edf5fa3c1c3168f7e53f68f54 Mon Sep 17 00:00:00 2001 From: yuj Date: Tue, 19 May 2026 12:18:22 +0800 Subject: [PATCH] fix(update): wait for writer 'finish' + honour backpressure + clean up tmp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `downloadFile()` in `src/update/self-update.ts` had three concrete reliability issues, all triggered in the self-update path: 1. `resolve()` was called as soon as the read loop hit `done`, but `writer.end()` only *queues* the close — bytes may still be in the kernel pagecache. The very next step, `verifySha256()`, reads back the same file; on slower disks (HDD, network FS, CI runners) this races the flush and surfaces as a spurious "Checksum mismatch" error mid-update. Replace the `resolve()` call with a `writer.on( 'finish')` handler so we only proceed once the file is durably closed. 2. `writer.write(value)` was called without honouring its boolean return value. For large binaries the writer's internal buffer grows without bound until the entire download is in memory. Wait for the `'drain'` event when `write()` returns `false`. 3. On any error (network reset, 5xx mid-stream, writer error), the half-written file at `dest` was left in `tmpdir()`. Unlink it in a catch block before rethrowing. 4. The Web Streams reader's lock was never released — neither on the happy path nor on errors. Move the `releaseLock()` into a `finally` so the underlying body stream is always freed. No behavior change on the success path beyond the (correct) flush wait. `tsc --noEmit` passes. --- src/update/self-update.ts | 49 ++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/update/self-update.ts b/src/update/self-update.ts index 1542b69..2cd8bf3 100644 --- a/src/update/self-update.ts +++ b/src/update/self-update.ts @@ -111,22 +111,39 @@ async function downloadFile(url: string, dest: string, onProgress?: (pct: number const writer = createWriteStream(dest); const reader = res.body!.getReader(); - await new Promise((resolve, reject) => { - writer.on('error', reject); - const pump = async () => { - try { - while (true) { - const { done, value } = await reader.read(); - if (done) { writer.end(); break; } - writer.write(value); - received += value.length; - if (onProgress && total > 0) onProgress(Math.round(received / total * 100)); - } - resolve(); - } catch (e) { reject(e); } - }; - pump(); - }); + try { + await new Promise((resolve, reject) => { + writer.on('error', reject); + // Resolve only after the writer has actually flushed and closed — + // otherwise verifySha256() can race the kernel's pagecache flush and + // produce spurious checksum mismatches on slow disks. + writer.on('finish', () => resolve()); + const pump = async () => { + try { + while (true) { + const { done, value } = await reader.read(); + if (done) { writer.end(); break; } + // Honour backpressure so we don't grow the writer's internal buffer + // unboundedly on large binaries / slow disks. + if (!writer.write(value)) { + await new Promise(r => writer.once('drain', r)); + } + received += value.length; + if (onProgress && total > 0) onProgress(Math.round(received / total * 100)); + } + } catch (e) { reject(e); } + }; + pump(); + }); + } catch (err) { + // Don't leave a half-downloaded binary in /tmp on failure. + try { (await import('fs')).unlinkSync(dest); } catch { /* best-effort */ } + throw err; + } finally { + // Always release the Web Streams reader lock — the API contract requires + // a paired acquire/release, and not doing so traps the underlying body. + reader.releaseLock(); + } } export async function resolveUpdateTarget(channel: Channel): Promise {