fix(upgrade): replace Bun.mmap with arrayBuffer on all platforms#343
Merged
fix(upgrade): replace Bun.mmap with arrayBuffer on all platforms#343
Conversation
Bun.mmap() always opens files with O_RDWR internally, which fails when the target is the currently-running executable: - macOS: AMFI sends uncatchable SIGKILL (writable mapping on signed Mach-O) - Linux: open() returns ETXTBSY (kernel blocks opening running executables for write) PR #340 added a platform-conditional (darwin→arrayBuffer, else→mmap) but this was insufficient — mmap fails on BOTH platforms. The { shared: false } flag only affects MAP_PRIVATE vs MAP_SHARED, not the O_RDWR open() call. Fix: use Bun.file().arrayBuffer() unconditionally on all platforms. Costs ~100 MB heap for the old binary (freed immediately after patching) but is the only approach that works cross-platform. Also improve delta upgrade error visibility: include the error message in the debug log so --verbose reveals the root cause.
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Trace
Other
Bug Fixes 🐛Api
Formatters
Setup
Upgrade
Other
Documentation 📚
Internal Changes 🔧Api
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ 2683 passed | Total: 2683 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 25.00%. Project has 3108 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 82.65% 82.64% -0.01%
==========================================
Files 127 127 —
Lines 17904 17902 -2
Branches 0 0 —
==========================================
+ Hits 14797 14794 -3
- Misses 3107 3108 +1
- Partials 0 0 —Generated by Codecov Action |
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
The old file (running binary) cannot be mmap'd directly — Bun always opens with O_RDWR, which fails on running executables (macOS: SIGKILL, Linux: ETXTBSY). PR #343 fixed this with arrayBuffer() (~100 MB JS heap). Improve: copy the old binary to a temp file first, then mmap the copy. The copy is a regular file with no running process, so mmap succeeds and the ~100 MB is kernel-managed (no GC pressure). On CoW-capable filesystems (btrfs, xfs, APFS) the copy is a metadata-only reflink — near-instant with zero extra disk I/O. Falls back to arrayBuffer() if copy or mmap fails for any reason.
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
The old file (running binary) cannot be mmap'd directly — Bun always opens with O_RDWR, which fails on running executables (macOS: SIGKILL, Linux: ETXTBSY). PR #343 fixed this with arrayBuffer() (~100 MB JS heap). Improve: copy the old binary to a temp file first, then mmap the copy. The copy is a regular file with no running process, so mmap succeeds and the ~100 MB is kernel-managed (no GC pressure). On CoW-capable filesystems (btrfs, xfs, APFS) the copy is a metadata-only reflink — near-instant with zero extra disk I/O. Falls back to arrayBuffer() if copy or mmap fails for any reason.
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
Delta upgrade failures were completely invisible in Sentry: - No spans for the delta attempt (only DB queries visible in traces) - Errors caught and logged at debug level (invisible without --verbose) - No captureException — errors never reported to Sentry This made it impossible to diagnose the ETXTBSY/SIGKILL issues (PRs #339, #340, #343) from telemetry alone — they were found through code analysis and local reproduction. Changes: - Wrap attemptDeltaUpgrade in withTracingSpan for a 'upgrade.delta' span - Record delta.from_version, delta.to_version, delta.channel as attributes - On success: record patch_bytes and sha256 prefix - On unavailable (no patch): record delta.result='unavailable' - On error: captureException with warning level + delta context tags, record delta.result='error' and delta.error message on span - Upgrade log.debug to log.warn for failure messages so users see them Now delta failures will appear as: 1. A span in the upgrade trace (with error status + attributes) 2. A warning-level exception in Sentry Issues (with delta context) 3. A visible stderr message to the user
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
Delta upgrade failures were completely invisible in Sentry: - No spans for the delta attempt (only DB queries visible in traces) - Errors caught and logged at debug level (invisible without --verbose) - No captureException — errors never reported to Sentry This made it impossible to diagnose the ETXTBSY/SIGKILL issues (PRs #339, #340, #343) from telemetry alone — they were found through code analysis and local reproduction. Changes: - Wrap attemptDeltaUpgrade in withTracingSpan for a 'upgrade.delta' span - Record delta.from_version, delta.to_version, delta.channel as attributes - On success: record patch_bytes and sha256 prefix - On unavailable (no patch): record delta.result='unavailable' - On error: captureException with warning level + delta context tags, record delta.result='error' and delta.error message on span - Upgrade log.debug to log.warn for failure messages so users see them Now delta failures will appear as: 1. A span in the upgrade trace (with error status + attributes) 2. A warning-level exception in Sentry Issues (with delta context) 3. A visible stderr message to the user
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
Delta upgrade failures were completely invisible in Sentry: - No spans for the delta attempt (only DB queries visible in traces) - Errors caught and logged at debug level (invisible without --verbose) - No captureException — errors never reported to Sentry This made it impossible to diagnose the ETXTBSY/SIGKILL issues (PRs #339, #340, #343) from telemetry alone — they were found through code analysis and local reproduction. Changes: - Wrap attemptDeltaUpgrade in withTracingSpan for a 'upgrade.delta' span - Record delta.from_version, delta.to_version, delta.channel as attributes - On success: record patch_bytes and sha256 prefix - On unavailable (no patch): record delta.result='unavailable' - On error: captureException with warning level + delta context tags, record delta.result='error' and delta.error message on span - Upgrade log.debug to log.warn for failure messages so users see them Now delta failures will appear as: 1. A span in the upgrade trace (with error status + attributes) 2. A warning-level exception in Sentry Issues (with delta context) 3. A visible stderr message to the user
BYK
added a commit
that referenced
this pull request
Mar 5, 2026
…ing (#344) ## Changes ### 1. Copy-then-mmap with child process probe for mmap safety `Bun.mmap()` on the running binary fails fatally on macOS (SIGKILL from AMFI) and Linux (ETXTBSY). PR #343 fixed this by using `arrayBuffer()` unconditionally, but that spikes ~100 MB onto the JS heap. This PR restores zero-heap mmap while handling the macOS SIGKILL: 1. **Copy**: `copyFileSync(process.execPath, tmpdir/sentry-patch-old-{pid})` CoW reflinks on btrfs/xfs/APFS for near-instant zero-I/O copy 2. **Probe**: Spawn a child process that tries `Bun.mmap(copy)` If macOS AMFI sends SIGKILL, only the child dies — parent survives and knows mmap is unsafe 3. **Mmap**: If probe exits 0, mmap the copy (zero JS heap, kernel-managed pages) 4. **Fallback**: If probe fails, read copy via `arrayBuffer()` (~100 MB heap) ### 2. Instrument delta upgrade with Sentry spans and error capture Delta failures were completely invisible in Sentry — no spans, no `captureException`, errors logged at debug level. The ETXTBSY/SIGKILL bugs (PRs #339–#343) were only discoverable through code analysis and local reproduction. - Wraps `attemptDeltaUpgrade` in `withTracingSpan('upgrade.delta')` - Records `delta.from_version`, `delta.to_version`, `delta.channel`, `delta.patch_bytes` - On error: `captureException` with warning level + delta context tags - Upgrades error log from `log.debug()` to `log.warn()` so users see failures
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Delta upgrades silently fall back to full binary download on Linux because
Bun.mmap()throwsETXTBSYwhen targeting the running executable.Bun.mmap()always opens files withO_RDWRinternally, regardless of thesharedflag. When the target file is the currently-running binary:open()returnsETXTBSY(kernel blocks opening running executables for write) — not fixed until nowPR #340 added a platform-conditional (
darwin→arrayBuffer, else →mmap) but this was insufficient since mmap fails on both platforms.Evidence
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 theBun.mmap()call, error is caught byattemptDeltaUpgrade()'s catch-all, and logged at debug level (invisible without--verbose).Fix
bspatch.ts, usenew Uint8Array(await Bun.file(oldPath).arrayBuffer())unconditionally on all platforms--verbosereveals root causeThe ~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