Drain an autorelease pool on the export worker threads - #297
Open
l7aromeo wants to merge 1 commit into
Open
Conversation
This was referenced Aug 8, 2026
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
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.
Drain an autorelease pool on the export worker threads
toBufferandsaveAshand their work torayon::spawn_fifo. On macOS that workallocates autoreleased Objective-C objects through Metal, and a rayon worker has no
autorelease pool of its own — so those objects are never drained and accumulate for the
life of the process.
The main thread doesn't have this problem because Node's event loop drains a pool each
tick, which is why the synchronous export methods are unaffected.
Evidence
heapon a process runningtoBufferin a loop, sampled 60 seconds apart. Exactly oneclass grows:
Everything else is flat. Meanwhile the malloc heap total stayed at ~17 MB while RSS
reached 432 MB, which is what you'd expect from allocations owned by the Metal driver
rather than by the allocator — and is why this doesn't show up under
leaksor under areplacement allocator.
MetalEngine::with_contextalready wraps its body inautoreleasepool, but the exportwork in
Page::encoded_as—surface.direct_context(),make_non_texture_image, theencoder calls — runs outside it.
The surcharge is Metal-only
If async export leaked for some general reason, it would leak on Vulkan too. It doesn't.
Same package, same version, no patch applied anywhere — only the backend differs:
The ~25 KiB/render present in every row is the unrelated full-canvas-fill leak (#296),
which is backend-independent. The surcharge on top of it appears only under Metal.
The same comparison on a fork that doesn't carry #296's leak isolates it further: 80.4
KiB/render on Metal versus −0.2 on Vulkan, from a flat baseline.
Vulkan was exercised on one Linux machine with NVIDIA and Intel ICDs, so this doesn't
prove every Vulkan driver is clean — only that the async path costs nothing extra there
while it costs ~77–82 KiB/render under Metal.
The fix
A helper in
src/gpu/mod.rs, gated on themetalfeature, plus a wrap at the twospawn_fifosites insrc/canvas.rs:Gating on
metalrather than on the target OS is deliberate on two counts:objcis anoptional dependency pulled in only by that feature, and the Metal engine is the only
thing in the crate that produces autoreleased objects, so the two conditions coincide.
On every other configuration this expands to the original expression, so non-Metal
builds are unchanged.
Measurements
Second-half RSS slope over 2000 renders, KiB per render:
toBufferSync, CPUtoBuffer, CPUtoBufferSync, GPUtoBuffer, GPUThis removes the entire GPU-async surcharge (~76 KiB/render) and touches nothing else.
The ~25 KiB/render floor that remains in every cell is a separate, unrelated leak — the
full-canvas fill optimization never firing — which I've opened separately as #296. The two
are independent and compose; with both applied all four cells sit at roughly −1
KiB/render.
Possible relation to #145
#145 reported async
toBufferleaking, was bisected to68bef1b, and was closedwithout a root cause. That commit only changes how Vulkan support is detected, which
flips the default rendering engine on some machines — it doesn't touch any export path.
That would explain why the bisect looked inexplicable: it changed which renderer users
landed on by default rather than introducing a leak.
I can't claim this fully explains #145 — the reporter saw sync-with-GPU as clean whereas
3.0.8 leaks ~25 KiB/render there, and that was a very different codebase — but it does
account for the async-specific component they couldn't pin down.
Testing
npm test— 141/141 pass.cargo clippy— no new warnings on the changed lines.cargo check --no-default-features— compiles, confirming the non-Metal path.main. Draining a pool per export could in principleinvalidate the
ImagethatPageCacheholds across calls, so I hashed the output ofrepeated exports on the same canvas —
toBuffer('png')followed bytoBuffer('jpg'),the same format twice, drawing more between exports (the partial-replay path with a
stale
cache_depth), interleaved sync/async, and 100 consecutive exports. All hashesmatch
mainon both CPU and GPU. Skia holds those textures viask_sp, so the poolpop only balances the autorelease, not the strong reference.
Caveats
to reproduce, so CI won't exercise it — the runners have no GPU and the suite has no
memory assertions.
toBuffer. The same change is applied to thesaveAs/write_sequencespawn site, which I did not separately measure.the original expression. Measured on Linux with a Vulkan-backed GPU to confirm that
backend has no comparable per-thread issue of its own (table above); that was one
machine, not a survey of drivers.
Reproducer
On this machine that prints
107.2 KiB/renderonmainand27.5 KiB/renderwith thepatch — the residue being the separate full-canvas-fill leak, which #296 takes to
roughly zero.
Run under
heap <pid>to watchAGXG16XFamilyBlitContextclimb onmainand holdsteady with this applied.