fix: destroy read streams to prevent file descriptor leaks#16002
Conversation
sirv.ts: createReadStream().pipe(res) leaked the file descriptor when
the HTTP response was closed prematurely (e.g. client disconnect). The
pipe does not automatically destroy the source stream when the
destination closes. Added res.on('close') handler to destroy the stream.
serve-app.ts: createReadStream() in readErrorPageFromDisk() was never
destroyed in the catch block. If the stream partially opened before the
error event fired (e.g. permission denied after open), the fd leaked
across retry iterations. Moved stream declaration outside try so it can
be destroyed in catch.
🦋 Changeset detectedLatest commit: 63a49d3 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| "astro": patch | ||
| --- | ||
|
|
||
| fix: destroy read streams to prevent file descriptor leaks |
There was a problem hiding this comment.
Please follow the contribution guide. Here's some tips for a good changeset https://contribute.docs.astro.build/docs-for-code-changes/changesets/#tips-and-examples
…e description per contribution guide Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thank you for the review and the pointer to the contribution guide -- I appreciate you taking the time. Apologies for the rough changeset on the first pass. I've updated it to scope the patch to I'll keep the changeset conventions in mind for future contributions -- scope to the right packages, write from the user's perspective, and drop conventional-commit prefixes from the description. |
Summary
Two
createReadStream()file descriptor leaks found via static analysis of the control flow graph:1.
packages/integrations/partytown/src/sirv.ts:129fs.createReadStream(file, opts).pipe(res)leaks the file descriptor when the HTTP response is closed prematurely (e.g. client disconnect, timeout). Node's.pipe()does not automatically destroy the source stream when the destination closes.Fix: Captured the stream reference and added
res.on('close', () => stream.destroy()).2.
packages/integrations/node/src/serve-app.ts:24createReadStream(fullPath)inreadErrorPageFromDisk()was never destroyed in thecatchblock. If the stream partially opened before the error event fired (e.g. permission change betweenopenanderror), the fd leaked across retry iterations as the loop tries the next file path pattern.Fix: Moved stream declaration outside
tryso it can be destroyed incatchviastream?.destroy().Test Plan
lsof -p <pid>