Skip to content

Commit 87bf5eb

Browse files
janhapkeclaude
andcommitted
Add patched sharp/libvips build fixing Electron Linux segfault
sharp/libvips crashes with a native segfault when decoding images inside any process built on Electron's Linux binary, caused by two independently-loaded copies of glib colliding in the same process (electron/electron#46323). This rebuilds libvips-cpp.so and sharp's native addon so the colliding glib/gobject symbols are hidden and re-exported under renamed wrappers instead, eliminating the collision at the linker level. Includes the full patch/build/test pipeline (Docker-only, scripted end-to-end via `npm run build`), cross-platform packaging (patched build on linux-x64, real sharp re-exported everywhere else), CI, and a guided /rebuild-for-version process for future sharp releases. Verified against sharp's own upstream test suite, a dedicated Electron crash-regression test, and end-to-end in a real consuming Electron application. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent bcc3f33 commit 87bf5eb

28 files changed

Lines changed: 3093 additions & 0 deletions
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
---
2+
description: Re-run this project's patch/build/test pipeline against a new sharp/sharp-libvips release, with judgment at each step rather than blind reapplication.
3+
argument-hint: <sharp-version> <sharp-libvips-version>
4+
---
5+
6+
# Rebuild for a new `sharp`/`sharp-libvips` version
7+
8+
Arguments: `$ARGUMENTS` — expected as `<sharp-version> <sharp-libvips-version>`, e.g. `0.36.0 1.4.0`. If either is missing, ask for it before doing anything else.
9+
10+
This is **not** a script to run to completion unattended — it's a guided process with real judgment calls at several points. Read all of this before starting, and stop to report back (rather than guessing and continuing) at any point marked **STOP**. The design rationale for why this is a command and not a shell script is in `README.md`'s "Updating to a new `sharp` release" section — read that first if this is the first time running this command.
11+
12+
## Step 0: Orient
13+
14+
- Read `patches/wrapper-symbols.json` — the current source of truth for which symbols are wrapped and why.
15+
- Read `README.md`'s "Engineering notes" section (especially "How the fix works" and "The wrapper symbol set") for the full context of what was done for the *current* target version and why.
16+
- Confirm the new target versions are real releases: check `https://github.com/lovell/sharp/releases` and `https://github.com/lovell/sharp-libvips/releases` (or the npm registry) for the tags given in `$ARGUMENTS`.
17+
18+
## Step 1: Check whether the bug still reproduces at all — before assuming any patches are needed
19+
20+
Do not skip this by assuming "same bug, same fix." Upstream could have partially or fully fixed this independently since the current target version.
21+
22+
1. Update the submodule pins: `git -C vendor/sharp-libvips fetch --tags && git -C vendor/sharp-libvips checkout v<sharp-libvips-version>`, same for `vendor/sharp`.
23+
2. Build **unpatched** — do *not* run `scripts/apply-patches.sh` yet. Just run `sharp-libvips`'s own `build.sh linux-x64` and `sharp`'s own default build (against the npm `@img/sharp-libvips-linux-x64` package, not `SHARP_FORCE_GLOBAL_LIBVIPS`) to get a genuinely stock addon.
24+
3. Run `test/electron-crash-repro.js` (with `SHARP_MODULE_PATH` pointed at this stock, unpatched `vendor/sharp`) via `ELECTRON_RUN_AS_NODE=1 electron ...`.
25+
4. **STOP and report** if it does *not* crash. Investigate why before proceeding — check `sharp-libvips`'s `build/posix.sh` for whether they broadened their own `vips.map` fix, check the [Electron tracking issue](https://github.com/electron/electron/issues/46323) for whether it's been closed, check whether `libvips`'s `VImage8.h` still has the same inline-header pattern. The patches in this repo might need to shrink, change shape entirely, or (best case) not be needed at all for this version. Do not proceed with the rest of this process on autopilot if this happens — the right next step depends entirely on *why* it's fixed.
26+
27+
If it still crashes (expected, most likely outcome): continue to Step 2.
28+
29+
## Step 2: Attempt to apply the existing patches
30+
31+
1. Run `scripts/apply-patches.sh`.
32+
2. **Clean apply**: proceed to Step 3.
33+
3. **Apply failure**: diagnose *why*, don't just retry blindly.
34+
- Pull up the specific hunk that failed. Diff the relevant upstream file between the *old* target tag (still readable via `git log`/`git show` in the submodule, or by checking out the previous tag in a scratch clone) and the *new* tag, focused on just the lines the patch touches.
35+
- Judge: is this a trivial context-line drift (safe to regenerate the patch mechanically — e.g. `git apply --recount` or a manual small edit to the patch file, then re-verify with `git apply --check`) or a structural change (e.g. `VImage8.h`'s `VObject` class reshaped, `build/posix.sh`'s libvips-build block reorganized, `cplusplus/meson.build`'s `library('vips-cpp', ...)` sources list restructured)?
36+
- For structural changes: **STOP and report** what changed and what you think the patch needs to become, before actually rewriting it. This is exactly the kind of judgment call that shouldn't happen silently.
37+
38+
## Step 3: Re-check the wrapper symbol scope
39+
40+
Do not assume the symbol set in `patches/wrapper-symbols.json` is still complete or still necessary.
41+
42+
1. Re-run the exhaustive grep for bare `glib`/`gobject` calls across *every* file in the new `vendor/sharp/src/` (not just the files that had them last time):
43+
```bash
44+
for f in $(git -C vendor/sharp ls-tree -r --name-only HEAD -- src/ | grep -E '\.(cc|h)$'); do
45+
git -C vendor/sharp show HEAD:"$f" | grep -noE '\bg_[a-z0-9_]+\s*\(' | while read -r m; do echo "$f: $m"; done
46+
done | sort -u
47+
```
48+
2. Also re-check whether `libvips`'s other public C++ headers (`VError8.h`, `VInterpolate8.h`, `VRegion8.h`, `VConnection8.h` — see `patches/wrapper-symbols.json`'s `headersChecked` for what was clean last time) have picked up the same inline-header pattern `VImage8.h` has.
49+
3. Diff both results against `patches/wrapper-symbols.json`. Report: any symbols now called that aren't wrapped (add them — see `extra/glib_wrapper.c`/`.h` in `vendor/sharp-libvips`, following the existing pattern, remembering the `__attribute__((visibility("default")))` requirement), and any wrapped symbols no longer called anywhere (safe to leave as-is; removing them is low priority and not required).
50+
4. Update `patches/wrapper-symbols.json` and the patch files to match — including the manifest's `targetVersions` field, which must always name the exact versions the manifest was verified against — then re-verify `git apply --check` on both.
51+
52+
## Step 4: Rebuild both phases
53+
54+
```bash
55+
npm run build:libvips
56+
npm run build:sharp
57+
```
58+
59+
## Step 5: Run both gates
60+
61+
```bash
62+
npm run test:gates
63+
```
64+
65+
**If either gate fails**, follow the same loop this project's own development actually used — this is expected to happen at least once, not a sign something is broken:
66+
67+
1. Identify the specific symbol from the failure. Gate 1 (sharp's test suite) failures that mention `undefined symbol: <name>` name it directly, as does gate 2's failure mode if it manifests the same way; a genuine segfault in gate 2 means a wrapped symbol still isn't taking effect somewhere (re-check `objdump -T` on the rebuilt `libvips-cpp.so` for that symbol in both directions before assuming it's a *new* symbol).
68+
2. Add it to the wrapper set (`extra/glib_wrapper.c`/`.h`, `patches/wrapper-symbols.json`, and the relevant call-site patch), regenerate both patch files, rebuild, re-run gates.
69+
3. **Bound this to 3 automatic iterations.** If gates still aren't green after 3 rounds, **STOP and report** a clear summary of what's still failing, rather than continuing to loop. Something more fundamental is likely different about this version.
70+
71+
## Step 6: Re-verify
72+
73+
1. `objdump -T` on the rebuilt `libvips-cpp.so`, checking **every** symbol in `patches/wrapper-symbols.json` in both directions (bare name absent, `vips_<name>` present as defined + global).
74+
2. `readelf -d` on the rebuilt `sharp.node` addon — confirm `DT_RPATH` (not `DT_RUNPATH`) once repackaged (see Step 7).
75+
3. `npm run package`.
76+
77+
## Step 7: Prepare (but do not execute) the release
78+
79+
```bash
80+
npm run release <new-version>-electron.0
81+
```
82+
83+
This runs the full pipeline again, bumps `package/package.json`'s version, and prints the remaining manual steps. **Do not run `npm publish` yourself, and do not push/tag without explicit confirmation.** Instead:
84+
85+
1. Write a `CHANGELOG.md` entry summarizing: the new upstream version, any patch adjustments made and why (link back to what Step 2 found), any wrapper-symbol changes (link back to what Step 3 found), and confirmation that both gates pass.
86+
2. **STOP and report** a summary to the user: what changed, what (if anything) needed judgment calls along the way, and that this is ready for their review before they decide to commit/tag/publish. Publishing is always an explicit, separate, human-confirmed action — never something this command does on its own, no matter how cleanly everything above went.

‎.github/workflows/ci.yml‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
pull_request:
6+
7+
jobs:
8+
# Runs the full Linux pipeline (patches -> Phase A -> Phase B -> both test
9+
# gates -> local packaging) on every push/PR. This is what actually keeps
10+
# README.md's claims honest going forward, rather than relying on
11+
# occasional manual runs.
12+
linux-build:
13+
runs-on: ubuntu-latest
14+
steps:
15+
- uses: actions/checkout@v7
16+
with:
17+
submodules: recursive
18+
19+
- uses: actions/setup-node@v6
20+
with:
21+
node-version: 24
22+
23+
- run: npm install
24+
25+
- name: Full build + both test gates + local packaging
26+
run: npm run build
27+
28+
- uses: actions/upload-artifact@v7
29+
if: always()
30+
with:
31+
name: package-linux-x64
32+
path: package/linux-x64
33+
if-no-files-found: ignore
34+
retention-days: 7
35+
36+
# Validates the non-Linux fallback (re-exporting real `sharp` unmodified)
37+
# for real, on real macOS/Windows runners — this path is otherwise
38+
# unverifiable on the maintainer's Linux-only dev machine. Deliberately
39+
# independent of the linux-build job above: doesn't need the Phase A/B
40+
# binaries at all, since the fallback branch never touches them.
41+
cross-platform-fallback:
42+
strategy:
43+
fail-fast: false
44+
matrix:
45+
os: [macos-13, macos-14, windows-2022]
46+
runs-on: ${{ matrix.os }}
47+
steps:
48+
- uses: actions/checkout@v7
49+
50+
- uses: actions/setup-node@v6
51+
with:
52+
node-version: 24
53+
54+
# Installs package/package.json's own "sharp" dependency (the real,
55+
# official npm package) — this is the one npm quirk documented in
56+
# README.md: an out-of-tree file: dependency doesn't get its
57+
# own dependencies installed automatically, so this has to happen
58+
# explicitly rather than relying on some outer npm install to do it.
59+
- name: Install package/'s own dependencies (real sharp, for the fallback)
60+
working-directory: package
61+
run: npm install --omit=dev
62+
63+
- name: Confirm the non-Linux fallback resolves to real sharp and works
64+
shell: bash
65+
run: |
66+
node -e "
67+
const sharp = require('./package/index.js');
68+
(async () => {
69+
console.log('platform:', process.platform, process.arch);
70+
console.log('sharp version:', sharp.versions.sharp);
71+
const raw = Buffer.alloc(64 * 64 * 3, 128);
72+
const jpeg = await sharp(raw, { raw: { width: 64, height: 64, channels: 3 } }).jpeg().toBuffer();
73+
console.log('encode ok:', jpeg.length, 'bytes');
74+
const meta = await sharp(jpeg).metadata();
75+
console.log('metadata ok:', meta.width, 'x', meta.height);
76+
const resized = await sharp(jpeg).resize(32, 32).toBuffer();
77+
console.log('resize ok:', resized.length, 'bytes');
78+
console.log('PASS');
79+
})().catch((err) => { console.error('FAIL', err); process.exit(1); });
80+
"

‎.gitignore‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
node_modules/
2+
dist/
3+
package/linux-x64/
4+
# generated at package time from the root README.md (npm shows the package's own README)
5+
package/README.md

‎.gitmodules‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
[submodule "vendor/sharp-libvips"]
2+
path = vendor/sharp-libvips
3+
url = https://github.com/lovell/sharp-libvips.git
4+
[submodule "vendor/sharp"]
5+
path = vendor/sharp
6+
url = https://github.com/lovell/sharp.git

‎CHANGELOG.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Changelog
2+
3+
## Unreleased
4+
5+
First release, targeting `sharp` `0.35.3` / `sharp-libvips` `1.3.2`.
6+
7+
- Rebuilds `libvips-cpp.so` with the full `glib`/`gobject` symbol surface hidden (upstream hides only `g_param_spec_types`), re-exporting the seven symbols `sharp`'s native addon actually needs under renamed `vips_g_*` wrappers — see `patches/wrapper-symbols.json` for the full list and per-symbol reasoning.
8+
- Rebuilds `sharp`'s native addon (and, transitively, `libvips`'s inline `VImage8.h` header) to call those wrappers instead of the now-hidden bare `glib` functions. This eliminates the symbol collision with Electron's own `glib` that segfaults image decoding on Linux ([electron/electron#46323](https://github.com/electron/electron/issues/46323)).
9+
- The compiled addon resolves its co-located `libvips-cpp.so` via old-style `DT_RPATH` (`$ORIGIN`), so it deterministically wins over any other copy reachable through the consumer's `LD_LIBRARY_PATH` or `node_modules`.
10+
- Cross-platform dispatch: `linux-x64` uses the patched build; any other Linux architecture throws a clear error at load time rather than silently using an unpatched build; macOS/Windows re-export the real, unmodified `sharp`.
11+
- Verified against `sharp`'s own upstream test suite (1804/1811 passing; the one failure is a pre-existing, unrelated Node ESM/CJS interop quirk that fails identically against stock `sharp`) and a dedicated Electron crash-regression test, plus end-to-end in a real consuming Electron application.
12+
- The full build pipeline is scripted and Docker-only (`npm run build`), with CI covering both the Linux pipeline and the macOS/Windows fallback path.
13+
- Ships TypeScript types (`package/index.d.ts`, forwarding to `sharp`'s own declarations) so the `overrides` install keeps full type information on both the patched and fallback paths.
14+
15+
Known gaps: CI has not yet produced a green run on real macOS/Windows runners, and `linux-arm64` has no patched build (it fails loudly rather than silently).

0 commit comments

Comments
 (0)