Memory leak in async get/set/list/remove: value buffer and async work are never freed
- 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