Repository navigation
fix(file): use the sync xattr API, the async path of fs-xattr leaks - #1357
Merged
ferhatelmas merged 2 commits intoSep 2, 2026
Merged
Conversation
ferhatelmas
force-pushed
the
fix/file-backend-xattr-leak
branch
from
September 2, 2026 18:21
bed5f18 to
c44f02c
Compare
ferhatelmas
approved these changes
Sep 2, 2026
Coverage Report for CI Build 33672207234Coverage decreased (-0.08%) to 81.766%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions17 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
fs-xattr's async path leaks about 0.5 kB per call: the value buffer, the per-call struct on error branches and the napi_async_work handle are never freed (fs-xattr#47, fix proposed upstream). Measured on this backend with production-shaped load: 0.46 to 0.59 kB per served file. The file backend reads two xattrs on every served object, so self-hosted instances grow unreclaimable memory all day (supabase#1349: 0.6 GB/day on a 1.3M requests/day instance). Until fs-xattr ships a fixed release, switch the file backend to the sync API, which does not leak. Reads are small metadata lookups on local files (cache-control, content-type, etag), so blocking is acceptable here. Method signatures are unchanged.
Signed-off-by: Ferhat Elmas <elmas.ferhat@gmail.com>
ferhatelmas
force-pushed
the
fix/file-backend-xattr-leak
branch
from
September 2, 2026 19:15
c44f02c to
4c51b85
Compare
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
3 participants
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.
Fixes #1349 on the storage-api side. The native fix itself is proposed upstream at LinusU/fs-xattr#48.
Credit for the diagnosis goes to @djonua, in this issue and in fs-xattr#47: every async fs-xattr call leaks the value buffer, the per-call struct on error branches, and the
napi_async_workhandle. The file backend reads two xattrs on every served object, so the leak lands straight on the serve path (their numbers: 0.46 kB per request, about 0.6 GB per day on a 1.3M requests per day instance).Until an fs-xattr release ships the upstream fix, this switches the file backend to the sync API, which does not leak:
getAttribute,setAttribute,removeAttributebecomegetAttributeSync,setAttributeSync,removeAttributeSyncinsrc/storage/backend/file.tsTradeoff, already discussed in #1349: the sync calls block the loop for the duration of a local
getxattr/setxattrsyscall. These are small metadata lookups on local files (cache-control, content-type, etag), and the reporter's own suggestion notes that this is acceptable at this size. If the upstream fix merges and releases, the async calls can come back and this change reverts cleanly.Tests: 30/30 in
file.test.ts. Full unit suite 1759/1760, the one failure (admin-app.test.ts, "registers shared Blob response handling") is pre-existing on master.