Skip to content

Commit 0da9cc9

Browse files
authored
fix(upgrade): replace Bun.mmap with arrayBuffer on all platforms (#343)
## Problem Delta upgrades silently fall back to full binary download on **Linux** because `Bun.mmap()` throws `ETXTBSY` when targeting the running executable. `Bun.mmap()` always opens files with `O_RDWR` internally, regardless of the `shared` flag. When the target file is the currently-running binary: - **macOS**: AMFI sends uncatchable SIGKILL (writable mapping on signed Mach-O) — fixed in PR #339/#340 - **Linux**: `open()` returns `ETXTBSY` (kernel blocks opening running executables for write) — **not fixed until now** PR #340 added a platform-conditional (`darwin` → `arrayBuffer`, else → `mmap`) but this was insufficient since mmap fails on **both** platforms. ### Evidence ```bash # mmap always fails on the running binary — even with MAP_PRIVATE $ bun -e 'Bun.mmap(process.execPath, { shared: false })' ETXTBSY: text file is busy, open # arrayBuffer works fine (opens with O_RDONLY) $ bun -e 'await Bun.file(process.execPath).arrayBuffer()' # ✓ reads ~100MB successfully ``` Confirmed via end-to-end test: chain resolution works perfectly (8 patch tags, 1-step chain, 19KB patch, SHA-256 matches), but `applyPatch()` fails on the `Bun.mmap()` call, error is caught by `attemptDeltaUpgrade()`'s catch-all, and logged at debug level (invisible without `--verbose`). ## Fix - Remove platform-conditional in `bspatch.ts`, use `new Uint8Array(await Bun.file(oldPath).arrayBuffer())` unconditionally on all platforms - Include error message in delta upgrade debug log so `--verbose` reveals root cause - Update JSDoc to reflect the arrayBuffer-only approach The ~100 MB heap cost is acceptable — it's freed immediately after patching completes, and the alternative (full ~32 MB gzipped download) is much slower. ### Performance comparison | Method | Download size | Total time | |--------|-------------|------------| | Full download | ~32 MB .gz | ~7.2s | | Delta patch | ~19 KB | ~0.85s |
1 parent 3cabb9b commit 0da9cc9

File tree

3 files changed

+22
-21
lines changed

3 files changed

+22
-21
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,7 @@ mock.module("./some-module", () => ({
666666
* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`.
667667
668668
<!-- lore:019cb963-cb63-722d-9365-b34336f4766d -->
669-
* **macOS SIGKILL on MAP\_SHARED mmap of signed Mach-O binaries**: macOS AMFI (code signing enforcement) sends uncatchable SIGKILL when \`Bun.mmap()\` is used on code-signed Mach-O binaries. \`Bun.mmap()\` always requests PROT\_WRITE regardless of the \`shared\` flag, and macOS rejects ANY writable mapping (MAP\_SHARED or MAP\_PRIVATE) on signed Mach-O. PR #339's MAP\_PRIVATE fix was insufficient. Fixed by replacing \`Bun.mmap(oldPath)\` with \`new Uint8Array(await Bun.file(oldPath).arrayBuffer())\` in bspatch.ts. Costs ~100 MB heap for the old binary but avoids the uncatchable SIGKILL entirely. Sentry telemetry confirmed: zero successful macOS upgrade traces from delta-enabled versions, while Linux worked fine.
669+
* **Bun.mmap() unusable on running executables on ANY platform**: \`Bun.mmap()\` always opens files with \`PROT\_WRITE\`/\`O\_RDWR\` regardless of the \`shared\` flag. This fails on the running binary: macOS sends uncatchable SIGKILL (AMFI rejects writable mappings on signed Mach-O), Linux returns ETXTBSY (kernel blocks opening running executables for write). The platform-conditional fix (PR #340: darwin→arrayBuffer, else→mmap) was insufficient — mmap fails on BOTH platforms. Fixed by using \`new Uint8Array(await Bun.file(oldPath).arrayBuffer())\` unconditionally in bspatch.ts. Costs ~100 MB heap for the old binary but is the only approach that works cross-platform.
670670
671671
<!-- lore:019c969a-1c90-7041-88a8-4e4d9a51ebed -->
672672
* **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`.

src/lib/bspatch.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,15 @@
55
* TRDIFF10 format (produced by zig-bsdiff with `--use-zstd`). Designed for
66
* minimal memory usage during CLI self-upgrades:
77
*
8-
* - Old binary: `Bun.mmap()` on Linux (0 JS heap), `arrayBuffer()` on macOS
8+
* - Old binary: loaded via `Bun.file().arrayBuffer()` (~100 MB heap)
99
* - Diff/extra blocks: streamed via `DecompressionStream('zstd')`
1010
* - Output: written incrementally to disk via `Bun.file().writer()`
1111
* - Integrity: SHA-256 computed inline via `Bun.CryptoHasher`
1212
*
13-
* Total heap usage: ~1-2 MB on Linux, ~100 MB on macOS (old file in memory).
13+
* Total heap usage: ~100 MB for old file + ~1-2 MB for streaming buffers.
14+
* `Bun.mmap()` is NOT usable here because the old file is the running binary:
15+
* - macOS: AMFI sends uncatchable SIGKILL (PROT_WRITE on signed Mach-O)
16+
* - Linux: ETXTBSY from `open()` with write flags on a running executable
1417
*
1518
* TRDIFF10 format (from zig-bsdiff):
1619
* ```
@@ -210,9 +213,9 @@ function createZstdStreamReader(compressed: Uint8Array): BufferedStreamReader {
210213
/**
211214
* Apply a TRDIFF10 binary patch with streaming I/O for minimal memory usage.
212215
*
213-
* Uses `Bun.mmap()` (Linux) or `arrayBuffer()` (macOS) for the old file, `DecompressionStream('zstd')`
214-
* for streaming diff/extra blocks (~16 KB buffers), `Bun.file().writer()`
215-
* for disk output, and `Bun.CryptoHasher` for inline SHA-256 verification.
216+
* Reads the old file into memory via `Bun.file().arrayBuffer()`, then streams
217+
* diff/extra blocks (~16 KB buffers) via `DecompressionStream('zstd')`,
218+
* writes output via `Bun.file().writer()`, and computes SHA-256 inline.
216219
*
217220
* @param oldPath - Path to the existing (old) binary file
218221
* @param patchData - Complete TRDIFF10 patch file contents
@@ -243,16 +246,12 @@ export async function applyPatch(
243246
);
244247
const extraReader = createZstdStreamReader(patchData.subarray(extraStart));
245248

246-
// On macOS, Bun.mmap() triggers an uncatchable SIGKILL from AMFI code
247-
// signing enforcement — it always requests PROT_WRITE, and macOS rejects
248-
// ANY writable mapping (MAP_SHARED or MAP_PRIVATE) on signed Mach-O
249-
// binaries. Fall back to reading into memory (~100 MB heap, freed after
250-
// patching). On Linux, mmap with MAP_PRIVATE is safe and avoids heap
251-
// allocation entirely (shared: false avoids ETXTBSY on the running binary).
252-
const oldFile =
253-
process.platform === "darwin"
254-
? new Uint8Array(await Bun.file(oldPath).arrayBuffer())
255-
: Bun.mmap(oldPath, { shared: false });
249+
// Bun.mmap() is NOT usable for the old file during self-upgrades because
250+
// it always opens with PROT_WRITE, and the old file is the running binary:
251+
// - macOS: AMFI sends uncatchable SIGKILL on writable mapping of signed Mach-O
252+
// - Linux: open() returns ETXTBSY when opening a running executable for write
253+
// Reading into memory costs ~100 MB heap but avoids both platform restrictions.
254+
const oldFile = new Uint8Array(await Bun.file(oldPath).arrayBuffer());
256255

257256
// Streaming output: write directly to disk, no output buffer in memory
258257
const writer = Bun.file(destPath).writer();

src/lib/delta-upgrade.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -579,9 +579,11 @@ export async function attemptDeltaUpgrade(
579579
return await resolveNightlyDelta(targetVersion, oldBinaryPath, destPath);
580580
}
581581
return await resolveStableDelta(targetVersion, oldBinaryPath, destPath);
582-
} catch {
583-
// Any error during delta upgrade → fall back to full download
584-
log.debug("Delta upgrade unavailable, falling back to full download");
582+
} catch (error) {
583+
// Any error during delta upgrade → fall back to full download.
584+
// Log at debug so --verbose reveals the root cause (ETXTBSY, network, etc.)
585+
const msg = error instanceof Error ? error.message : String(error);
586+
log.debug(`Delta upgrade failed (${msg}), falling back to full download`);
585587
return null;
586588
}
587589
}
@@ -671,8 +673,8 @@ function cleanupIntermediates(destPath: string): void {
671673
*
672674
* For single-patch chains, applies directly from old binary to dest.
673675
* For multi-patch chains, alternates between two intermediate files
674-
* so that read (mmap) and write never target the same path — writing
675-
* to the mmap source would truncate it and corrupt the output.
676+
* so that read and write never target the same path — writing to the
677+
* source would truncate it and corrupt the output.
676678
*
677679
* Does **not** set executable permissions — the caller
678680
* (`downloadBinaryToTemp`) handles that uniformly for both delta

0 commit comments

Comments
 (0)