LinusU / LinusU/fs-xattr

Memory leak in async get/set/list/remove: value buffer and async work are never freed

Open
#47 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
71
Forks
20
PR merge metrics
No merged PRs in 30d

Description

The async code path leaks on every call. In `src/async.c`:

- `xattr_get_execute` allocates `data->value = malloc(...)` (line 34).
- `xattr_get_complete` copies it into a JS buffer via `napi_create_buffer_copy`
(line 62) and then frees only `_data` (line 65) — **`data->value` is never freed**.
- The error branch (lines 54-58) returns early **without** `free(_data)`, leaking
the whole struct plus `filename`/`attribute` are freed but the struct is not.
- `napi_delete_async_work` is never called — there are four `napi_queue_async_work`
calls and zero deletions, so every call also leaks a `napi_async_work` object.

The sync path (`src/sync.c`) is correct and does not leak, which isolates the bug
to the async implementation.

### Reproduction

```js
const xattr = require('fs-xattr')
const fs = require('fs')
const rss = () => (Number(fs.readFileSync('/proc/self/status','utf8').match(/VmRSS:\s+(\d+)/)[1])/1024).toFixed(1)
;(async () => {
console.log('start: ' + rss() + ' MB')
for (let r = 1; r <= 5; r++) {
for (let i = 0; i < 200000; i++) await xattr.get('./somefile', 'user.test')
console.log(r * 200000 + ' calls: ' + rss() + ' MB')
}
})()
```

Node 24.14.0, Alpine 3.x (musl), fs-xattr 0.3.1 and 0.4.0 — identical results:

| calls | async `get` | sync `getSync` | async, patched |
|---|---|---|---|
| 0 | 48.2 MB | 48.4 MB | 48.4 MB |
| 200k | 158.0 MB | 48.5 MB | 55.7 MB |
| 600k | 342.6 MB | 48.6 MB | 55.7 MB |
| **1M** | **497.6 MB** | 56.7 MB | **55.7 MB** |

That is **~0.46 kB leaked per call**, growing linearly with no plateau. V8 heap also
grows (122 MB vs 6.8 MB after the fix) because the `napi_async_work` objects are
retained too.

### Impact

This is not theoretical. `supabase/storage-api` calls `xattr.get` once per served
file on the file backend. On our instance (1.3M image requests/day) it produced
**~0.6 GB/day** of unreclaimable native memory; the container sat pinned at 98% of
its 3 GB limit, hitting the cgroup ceiling 182,905 times and continuously evicting
its own page cache.

### Fix

Four changes in `src/async.c`, verified by the table above:

1. `free(data->value)` in `xattr_get_complete` (both branches; initialise
`data->value = NULL` at allocation so the error path is safe).
2. `free(_data)` on the error branch.
3. Store the work handle in the data struct (`napi_async_work work;`) and pass
`&data->work` to `napi_create_async_work`.
4. `napi_delete_async_work(env, data->work)` before `free(_data)` in all four
`*_complete` handlers.

Happy to open a PR if useful.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/async.c and compare its four completion handlers with the leak-free path in src/sync.c. Use the provided Node.js reproduction to measure memory across repeated async calls. Done means the value buffer, error-path data, and async work are released, with memory remaining near the patched results.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, node.js
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.