From c9c91c80f4c4c1cb4e83c435d9800bd042b35353 Mon Sep 17 00:00:00 2001 From: Guillaume Flambard Date: Wed, 2 Sep 2026 10:02:43 +0200 Subject: [PATCH 1/2] fix(file): use the sync xattr API, the async path of fs-xattr leaks 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/storage-api#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. --- src/storage/backend/file.test.ts | 68 ++++++++++++++++++-------------- src/storage/backend/file.ts | 11 ++++-- 2 files changed, 46 insertions(+), 33 deletions(-) diff --git a/src/storage/backend/file.test.ts b/src/storage/backend/file.test.ts index 28b6b85ea..6e30ef318 100644 --- a/src/storage/backend/file.test.ts +++ b/src/storage/backend/file.test.ts @@ -12,9 +12,9 @@ import { withOptionalVersion } from './adapter' import { FileBackend } from './file' vi.mock('fs-xattr', () => ({ - setAttribute: vi.fn(() => Promise.resolve()), - getAttribute: vi.fn(() => Promise.resolve(undefined)), - removeAttribute: vi.fn(() => Promise.resolve()), + setAttributeSync: vi.fn(() => undefined), + getAttributeSync: vi.fn(() => undefined), + removeAttributeSync: vi.fn(() => undefined), })) describe('FileBackend xattr metadata', () => { @@ -48,7 +48,7 @@ describe('FileBackend xattr metadata', () => { await backend.uploadPart('bucket', 'key', 'v1', uploadId as string, 1, Readable.from('hello')) - expect(xattr.setAttribute).toHaveBeenCalledWith( + expect(xattr.setAttributeSync).toHaveBeenCalledWith( expect.any(String), 'user.supabase.etag', expect.any(String) @@ -107,12 +107,12 @@ describe('FileBackend xattr metadata', () => { await fsp.mkdir(partDir, { recursive: true }) await fsp.writeFile(partPath, 'hello') - const xattrGet = xattr.getAttribute as unknown as Mock + const xattrGet = xattr.getAttributeSync as unknown as Mock xattrGet.mockImplementation((_file: string, attribute: string) => { if (attribute === 'user.supabase.etag') { - return Promise.resolve(Buffer.from('part-etag')) + return Buffer.from('part-etag') } - return Promise.resolve(undefined) + return undefined }) uploadSpy = vi @@ -142,7 +142,7 @@ describe('FileBackend xattr metadata', () => { ETag: '"final"', }) - expect(xattr.getAttribute).toHaveBeenCalledWith(expect.any(String), 'user.supabase.etag') + expect(xattr.getAttributeSync).toHaveBeenCalledWith(expect.any(String), 'user.supabase.etag') } finally { uploadSpy?.mockRestore() if (originalPlatformDescriptor) { @@ -419,19 +419,19 @@ describe('FileBackend copy metadata options', () => { getConfig({ reload: true }) backend = new FileBackend() - const xattrGet = xattr.getAttribute as unknown as Mock + const xattrGet = xattr.getAttributeSync as unknown as Mock xattrGet.mockReset() xattrGet.mockImplementation((_file: string, attribute: string) => { if (attribute === 'user.supabase.cache-control') { - return Promise.resolve(Buffer.from('max-age=60')) + return Buffer.from('max-age=60') } if (attribute === 'user.supabase.content-type') { - return Promise.resolve(Buffer.from('text/plain')) + return Buffer.from('text/plain') } - return Promise.resolve(undefined) + return undefined }) - ;(xattr.setAttribute as unknown as Mock).mockReset().mockResolvedValue(undefined) - ;(xattr.removeAttribute as unknown as Mock).mockReset().mockResolvedValue(undefined) + ;(xattr.setAttributeSync as unknown as Mock).mockReset() + ;(xattr.removeAttributeSync as unknown as Mock).mockReset() await backend.uploadObject( 'bucket', @@ -441,14 +441,14 @@ describe('FileBackend copy metadata options', () => { 'text/plain', 'max-age=60' ) - ;(xattr.setAttribute as unknown as Mock).mockClear() - ;(xattr.removeAttribute as unknown as Mock).mockClear() + ;(xattr.setAttributeSync as unknown as Mock).mockClear() + ;(xattr.removeAttributeSync as unknown as Mock).mockClear() }) afterEach(async () => { - ;(xattr.getAttribute as unknown as Mock).mockReset().mockResolvedValue(undefined) - ;(xattr.setAttribute as unknown as Mock).mockReset().mockResolvedValue(undefined) - ;(xattr.removeAttribute as unknown as Mock).mockReset().mockResolvedValue(undefined) + ;(xattr.getAttributeSync as unknown as Mock).mockReset() + ;(xattr.setAttributeSync as unknown as Mock).mockReset() + ;(xattr.removeAttributeSync as unknown as Mock).mockReset() if (originalPlatformDescriptor) { Object.defineProperty(process, 'platform', originalPlatformDescriptor) } @@ -531,12 +531,12 @@ describe('FileBackend copy metadata options', () => { cacheControl: 'max-age=999', contentType: undefined, }) - expect(xattr.setAttribute).toHaveBeenCalledWith( + expect(xattr.setAttributeSync).toHaveBeenCalledWith( expect.any(String), 'user.supabase.cache-control', 'max-age=999' ) - expect(xattr.removeAttribute).toHaveBeenCalledWith( + expect(xattr.removeAttributeSync).toHaveBeenCalledWith( expect.any(String), 'user.supabase.content-type' ) @@ -554,13 +554,13 @@ describe('FileBackend copy metadata options', () => { { copyMetadata: false } ) - expect(xattr.setAttribute).not.toHaveBeenCalled() - expect(xattr.removeAttribute).toHaveBeenCalledTimes(2) - expect(xattr.removeAttribute).toHaveBeenCalledWith( + expect(xattr.setAttributeSync).not.toHaveBeenCalled() + expect(xattr.removeAttributeSync).toHaveBeenCalledTimes(2) + expect(xattr.removeAttributeSync).toHaveBeenCalledWith( expect.any(String), 'user.supabase.cache-control' ) - expect(xattr.removeAttribute).toHaveBeenCalledWith( + expect(xattr.removeAttributeSync).toHaveBeenCalledWith( expect.any(String), 'user.supabase.content-type' ) @@ -568,7 +568,9 @@ describe('FileBackend copy metadata options', () => { it('preserves absent source metadata when copyMetadata is true', async () => { const missingXattr = Object.assign(new Error('missing xattr'), { code: 'ENODATA' }) - ;(xattr.getAttribute as unknown as Mock).mockRejectedValue(missingXattr) + ;(xattr.getAttributeSync as unknown as Mock).mockImplementation(() => { + throw missingXattr + }) await expect( backend.copyObject( @@ -583,12 +585,14 @@ describe('FileBackend copy metadata options', () => { ) ).resolves.toMatchObject({ httpStatusCode: 200 }) - expect(xattr.removeAttribute).toHaveBeenCalledTimes(2) + expect(xattr.removeAttributeSync).toHaveBeenCalledTimes(2) }) it('ignores already absent destination metadata', async () => { const missingXattr = Object.assign(new Error('missing xattr'), { code: 'ENOATTR' }) - ;(xattr.removeAttribute as unknown as Mock).mockRejectedValue(missingXattr) + ;(xattr.removeAttributeSync as unknown as Mock).mockImplementation(() => { + throw missingXattr + }) await expect( backend.copyObject( @@ -606,7 +610,9 @@ describe('FileBackend copy metadata options', () => { it('propagates genuine source metadata read errors', async () => { const readError = Object.assign(new Error('xattr read failed'), { code: 'EIO' }) - ;(xattr.getAttribute as unknown as Mock).mockRejectedValue(readError) + ;(xattr.getAttributeSync as unknown as Mock).mockImplementation(() => { + throw readError + }) await expect( backend.copyObject( @@ -624,7 +630,9 @@ describe('FileBackend copy metadata options', () => { it('propagates genuine destination metadata removal errors', async () => { const removeError = Object.assign(new Error('xattr removal failed'), { code: 'EIO' }) - ;(xattr.removeAttribute as unknown as Mock).mockRejectedValue(removeError) + ;(xattr.removeAttributeSync as unknown as Mock).mockImplementation(() => { + throw removeError + }) await expect( backend.copyObject( diff --git a/src/storage/backend/file.ts b/src/storage/backend/file.ts index cc253dea3..a7c542887 100644 --- a/src/storage/backend/file.ts +++ b/src/storage/backend/file.ts @@ -608,7 +608,10 @@ export class FileBackend implements StorageBackendAdapter { protected async getMetadataAttr(file: string, attribute: string): Promise { try { - const value = await xattr.getAttribute(file, attribute) + // fs-xattr's async path leaks about 0.5 kB per call (fs-xattr#47, reported + // for this backend in #1349). The sync path does not leak and the reads + // are small, so it is used until an upstream release ships the fix. + const value = xattr.getAttributeSync(file, attribute) return value?.toString() ?? undefined } catch (error) { if (isMissingXattrError(error)) { @@ -619,7 +622,9 @@ export class FileBackend implements StorageBackendAdapter { } protected setMetadataAttr(file: string, attribute: string, value: string): Promise { - return xattr.setAttribute(file, attribute, value) + // see getMetadataAttr: sync xattr calls until fs-xattr ships a fixed release + xattr.setAttributeSync(file, attribute, value) + return Promise.resolve() } protected async setOrRemoveMetadataAttr( @@ -633,7 +638,7 @@ export class FileBackend implements StorageBackendAdapter { } try { - await xattr.removeAttribute(file, attribute) + xattr.removeAttributeSync(file, attribute) } catch (error) { if (!isMissingXattrError(error)) { throw error From 4c51b85bc4d2573357429387f29c10fee83ab4c8 Mon Sep 17 00:00:00 2001 From: Ferhat Elmas Date: Wed, 2 Sep 2026 21:19:35 +0300 Subject: [PATCH 2/2] fix: handle async context Signed-off-by: Ferhat Elmas --- src/storage/backend/file.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/storage/backend/file.ts b/src/storage/backend/file.ts index a7c542887..9385972ec 100644 --- a/src/storage/backend/file.ts +++ b/src/storage/backend/file.ts @@ -621,10 +621,9 @@ export class FileBackend implements StorageBackendAdapter { } } - protected setMetadataAttr(file: string, attribute: string, value: string): Promise { + protected async setMetadataAttr(file: string, attribute: string, value: string): Promise { // see getMetadataAttr: sync xattr calls until fs-xattr ships a fixed release xattr.setAttributeSync(file, attribute, value) - return Promise.resolve() } protected async setOrRemoveMetadataAttr(