fix: encodeViaTurboStream leaked memory via unremoved AbortSignal listener

[jacob-ebey]

See #14891

[changeset-bot]

🦋 Changeset detected

Latest commit: 86fdf73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
react-router	Patch
@react-router/architect	Patch
@react-router/cloudflare	Patch
@react-router/dev	Patch
react-router-dom	Patch
@react-router/express	Patch
@react-router/node	Patch
@react-router/serve	Patch
@react-router/fs-routes	Patch
@react-router/remix-routes-option-adapter	Patch
create-react-router	Patch

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

[rossipedia]

IMO this is just a touch over-complicated. Using { once: true } should have the same effect and be much more straightforward to grok:

Ok scratch that, after chatting in Discord about it I think something like this is the right approach:

diff --git a/packages/react-router/lib/server-runtime/single-fetch.ts b/packages/react-router/lib/server-runtime/single-fetch.ts
index aecd30970..9f929fdc6 100644
--- a/packages/react-router/lib/server-runtime/single-fetch.ts
+++ b/packages/react-router/lib/server-runtime/single-fetch.ts
@@ -390,14 +390,18 @@ export function encodeViaTurboStream(
   // through React's rendering stream before we call `abort()` on that.  If the
   // user provides their own it's up to them to decouple the aborting of the
   // stream from the aborting of React's `renderToPipeableStream`
+  let clearStreamTimeout: (() => void) | undefined;
   let timeoutId = setTimeout(
-    () => controller.abort(new Error("Server Timeout")),
+    () => {
+      controller.abort(new Error("Server Timeout"));
+      clearStreamTimeout();
+    },
     typeof streamTimeout === "number" ? streamTimeout : 4950,
   );
 
-  let clearStreamTimeout = () => clearTimeout(timeoutId);
+  clearStreamTimeout = () => clearTimeout(timeoutId);
 
-  requestSignal.addEventListener("abort", clearStreamTimeout);
+  requestSignal.addEventListener("abort", clearStreamTimeout, { once: true });
 
   return encode(data, {
     signal: controller.signal,

[mcollina]

Note: I did not experience any leaks that required this change when I measured, so possibly this required some other conditions to happen.

What I wanted to do with the original PR was to make it collectable immediately, instead of waiting 5 seconds and make it move to old space.

Great improvement.

[github-actions]

🤖 Hello there,

We just published version 7.14.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 7.14.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!