Skip to content

deps: v8 backport 9689b17687b#37865

Closed
guybedford wants to merge 1 commit intonodejs:v14.x-stagingfrom
guybedford:v8-cycle-fix-backport
Closed

deps: v8 backport 9689b17687b#37865
guybedford wants to merge 1 commit intonodejs:v14.x-stagingfrom
guybedford:v8-cycle-fix-backport

Conversation

@guybedford
Copy link
Contributor

@guybedford guybedford commented Mar 22, 2021

Original commit from https://chromium-review.googlesource.com/c/v8/v8/+/2667772, fixing a TLA execution ordering spec bug:

[top-level-await] Implement spec fix for cycle root detection

There is a bug in the top-level await spec draft such that async
strongly connected components are not always evaluated before their
depending modules.

See https://github.com/tc39/proposal-top-level-await/pull/161 for full
discussion and spec fix.

Bug: v8:11376
Change-Id: I88bf06afb2e9a5d8d0b757de8276f1d1242a875e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2667772
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72508}

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v14.x v8 engine Issues and PRs related to the V8 dependency. labels Mar 22, 2021
@guybedford guybedford requested a review from targos March 22, 2021 16:50
@targos
Copy link
Member

targos commented Mar 22, 2021

Please add Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0 to the commit message

@targos
Copy link
Member

targos commented Mar 22, 2021

Note to whoever lands this: the embedder string should be incremented.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 22, 2021 •
edited by targos
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/36909/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/3862/

@aduh95
Copy link
Contributor

aduh95 commented Mar 22, 2021

This is targeting v14.x. Did you mean to target v14.x-staging or master instead?

@targos
Copy link
Member

targos commented Mar 23, 2021

master already contains the commit. I guess it's okay to target v14.x. We just need to land on staging.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 24, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36944/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 25, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36977/

@guybedford
Copy link
Contributor Author

guybedford commented Mar 25, 2021

It would be great to get another approval here to land this TLA bug fix for 14.x.

@guybedford guybedford changed the base branch from v14.x to v14.x-staging March 29, 2021 21:25
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 29, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/37043/

@guybedford
Copy link
Contributor Author

guybedford commented Mar 29, 2021

Switched the branch to rerun CI on 14.x-staging. Will land tomorrow.

guybedford added a commit that referenced this pull request Mar 29, 2021
@guybedford
deps: v8 backport 9689b17687b
fe5e2e3 
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@guybedford
Copy link
Contributor Author

guybedford commented Mar 29, 2021

Landed in fe5e2e3.

@guybedford guybedford closed this Mar 29, 2021
guybedford added a commit to guybedford/node that referenced this pull request Mar 30, 2021
@guybedford
deps: v8 backport 9689b17687b
abe04be 
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: nodejs#37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@guybedford guybedford mentioned this pull request Mar 30, 2021
Closed
ruyadorno pushed a commit that referenced this pull request Mar 31, 2021
@guybedford @ruyadorno
deps: v8 backport 9689b17687b
2fd97ce 
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Backport-PR-URL: #37985
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 6, 2021
@guybedford @MylesBorins
deps: v8 backport 9689b17687b
a92ecf0 
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targos targos targos approved these changes

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@guybedford @targos @nodejs-github-bot @aduh95