deps: v8: Make Promise resolvers not keep resolved Promises alive#64101
Open
bakkot wants to merge 3 commits into
Open
deps: v8: Make Promise resolvers not keep resolved Promises alive#64101bakkot wants to merge 3 commits into
bakkot wants to merge 3 commits into
Conversation
Original commit message:
[api] Deprecate kPromiseRejectAfterResolved and kPromiseResolveAfterResolved
These events will be removed soon.
Bug: 42213031
Change-Id: Ie70474ff33c40c7d9cb0c2d0fbe6b75da3c53a22
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7774767
Commit-Queue: Kevin Gibbons <bakkot@gmail.com>
Reviewed-by: Olivier Flückiger <olivf@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#107395}
Refs: v8/v8@1a391f9
Original commit message:
[api] Remove PromiseResolveAfterResolved and PromiseRejectAfterResolved
These were previously deprecated in https://crrev.com/c/7774767
Bug: 42213031
Change-Id: I07b802b743bf052611f30a61c1132231df22f0bd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7897881
Commit-Queue: Olivier Flückiger <olivf@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Olivier Flückiger <olivf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108040}
Refs: v8/v8@0cc9eb2
Original commit message:
[builtins] Make Promise resolvers not keep resolved Promises alive
Currently a Promise's resolve/reject functions unconditionally hold the
Promise, plus a separate bit to track whether the Promise has been
resolved. Instead, hold a single field with Promise|Undefined.
This allows GC'ing a resolved Promise even if its resolvers are still
alive.
R=olivf@chromium.org
Fixed: 42213031
Change-Id: Ice645dbabb79e63dfcac8ae843cad95439d7a1f1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7646250
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Commit-Queue: Kevin Gibbons <bakkot@gmail.com>
Reviewed-by: Olivier Flückiger <olivf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#108183}
Refs: v8/v8@da20a19
Co-authored-by: Kevin Gibbons <bakkot@gmail.com>
Collaborator
|
Review requested:
|
nodejs-github-bot
added
build
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of 7774767, 7897881, and 7646250, the last of which just landed a few hours ago but I don't anticipate problems with it. It wasn't quite trivial because upstream v8 introduced a new use of the
kPromiseSlotwhich node's copy doesn't have yet, so there is a single upstream hunk which is left out here. Otherwise this is just the result ofgit node v8 backport 1a391f98cc7a9196369f2d6cab7df35ffbe92c08 0cc9eb22c0b0d132344b83b906f8a0e5aaa7b61e da20a197a7f9c2045b1ee501a472072944a86332.This fixes (or at least seriously mitigates) #17469 - not quite a complete fix because there's still two resolvers created and held per call to
Promise.race, but the result Promise itself is not held, so you aren't holding on to arbitrarily large objects like this.Node used to depend on the APIs removed herein, but they were deprecated ages ago and EOL'd in #58707, with the last traces removed in #62336.