Fix memory leak from long-lived cancellation promise

[kentonv]

This fix is actually #154 from @VastBlast, but I removed the middle commit and squashed the other two. (The middle commit of #154 was addressing an unrelated problem and is more controversial -- see my comments there.)

This fixes a memory leak in long sessions. It turns out that due to the implementation details of JavaScript promises:

• If you call Promise.race([a, b]) many times and one of the promises (b, let's say) is always the same promise, then each call will add a new "reaction" callback to b, which never gets removed until b actually fires. These reactions all become no-ops assuming a resolves first, but they still stick around.
• Worse, the reaction itself holds a reference to the Promise representing the result of the race, and that Promise object itself, once resolved, holds a reference to its resolution. Hence, all of the resolutions of the different a promises end up reachable from b and so cannot be GC'd.

The net result for Cap'n Web is: All of the messages received on a WebSocket session remained stuck in-memory until the session closed. Whoops.

This is all very silly. One would expect that a reactor callback pointing to an already-resolved promise should simply "null itself out" so that the GC can see that the resolution is not reachable through it. But the problem seems to have arisen because:

• The language spec does not specify how GC should work, so does not concern itself with anything to do with making GC work better.
• Hence, the spec does not specify that resolver functions should "null themselves out" -- though it also doesn't explicitly say they can't.
• Implementations seem to have implemented the spec to the letter, without any additional thought about GC.

Ironically, despite this problem existing for a decade already, it seems there is a recent push to fix it, at least in V8: https://chromium-review.googlesource.com/c/v8/v8/+/7646250

But this won't fix the Promise.race leak (first point above), so we should still land this fix to Cap'n WEb.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5954fbd

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

This PR includes changesets to release 1 package

Name	Type
capnweb	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

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@168

commit: 5954fbd

[VastBlast]

Thanks for the fix and taking over! And sorry about the delay in response, I’ve been busy past few weeks and couldn’t find the time to apply the requested changes.