Fix potential EBADF error when restarting workerd process in Miniflare #12025
Conversation
Explicitly destroy all stdio streams (stdin, stdout, stderr, and control pipe) before killing the workerd process in Runtime#dispose(). This ensures file descriptors are properly released before spawning a new process, preventing EBADF errors during rapid restarts. Fixes #11675
🦋 Changeset detectedLatest commit: d9ed3ad 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 |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
| runtimeProcess.stdin?.destroy(); | ||
| runtimeProcess.stdout?.destroy(); | ||
| runtimeProcess.stderr?.destroy(); | ||
| // The control pipe at stdio[3] is a Readable stream |
There was a problem hiding this comment.
That's not consistent with l335?
There was a problem hiding this comment.
I guess this could be:
| // The control pipe at stdio[3] is a Readable stream | |
| // The control pipe at stdio[3] could be a Readable stream. If so also destroy it |
| }); | ||
|
|
||
| test("Miniflare: setOptions: can restart workerd multiple times in succession", async () => { | ||
| // Regression test for https://github.com/cloudflare/workers-sdk/issues/11675 |
There was a problem hiding this comment.
annotate ?
There was a problem hiding this comment.
I think annotations are too noisy for this sort of thing. We don't need this to be displayed in every test run report.
There was a problem hiding this comment.
https://vitest.dev/guide/test-annotations.html#built-in-reporters
The default reporter prints annotations only if the test has failed
There was a problem hiding this comment.
Yes but I regularly use verbose reporter.
There was a problem hiding this comment.
And more importantly, this link to the original issue is not needed in the reporter outpiut. If the test fails it is because something has broken that needs fixing. The original issue is only one of the sources for looking into what could be the cause.
2 participants
Explicitly destroy all stdio streams (stdin, stdout, stderr, and control pipe) before killing the workerd process in Runtime#dispose(). This ensures file descriptors are properly released before spawning a new process, preventing EBADF errors during rapid restarts.
The original issue was not able to provide a reproduction that we could test so this PR is speculative.
Fixes #11675
A picture of a cute animal (not mandatory, but encouraged)