Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

deps: V8: backport ff8d67c88449 #44423

Merged
merged 1 commit into from
Sep 1, 2022
Merged

deps: V8: backport ff8d67c88449 #44423

merged 1 commit into from
Sep 1, 2022

Conversation

targos
Copy link
Member

@targos targos commented Aug 28, 2022

Original commit message:

Reland "Fix Context PromiseHook behaviour with debugger enabled"

This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

Original change's description:
> Fix Context PromiseHook behaviour with debugger enabled
>
> This is a solution for https://github.com/nodejs/node/issues/43148.
>
> Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
>
> Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
> Reviewed-by: Victor Gomes <victorgomes@chromium.org>
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82132}

Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: #43148
Fixes: #44415

@targos
deps: V8: backport ff8d67c88449
72b15c9 
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for nodejs#43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: nodejs#43148
Fixes: nodejs#44415
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@targos targos mentioned this pull request Aug 28, 2022
Closed
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Aug 28, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 28, 2022 •
edited by targos
Loading

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

Qard
Qard approved these changes Aug 28, 2022
View reviewed changes
Copy link
Member

@Qard Qard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing the backport for me! I was meaning to get to it, but things have been a bit chaotic for me recently. 😬

@nodejs-github-bot
Copy link
Collaborator

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

This was referenced Aug 31, 2022
Open
Open
@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 1, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 1, 2022
@nodejs-github-bot nodejs-github-bot merged commit 767a78a into nodejs:main Sep 1, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 767a78a

@targos targos deleted the fix-44415 branch September 1, 2022 08:23
@github-actions github-actions bot mentioned this pull request Sep 2, 2022
Open
24 tasks
@mrbbot mrbbot mentioned this pull request Sep 3, 2022
Closed
RafaelGSS pushed a commit that referenced this pull request Sep 5, 2022
@targos @RafaelGSS
deps: V8: backport ff8d67c88449
7aaca98 
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for #43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: #43148
Fixes: #44415
PR-URL: #44423
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@RafaelGSS RafaelGSS mentioned this pull request Sep 5, 2022
Merged
RafaelGSS pushed a commit that referenced this pull request Sep 6, 2022
@targos @RafaelGSS
deps: V8: backport ff8d67c88449
2d6525b 
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for #43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: #43148
Fixes: #44415
PR-URL: #44423
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
RafaelGSS pushed a commit that referenced this pull request Sep 7, 2022
@targos @RafaelGSS
deps: V8: backport ff8d67c88449
c5630ad 
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for #43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: #43148
Fixes: #44415
PR-URL: #44423
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
@targos @Fyko
deps: V8: backport ff8d67c88449
d54ee5a 
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for nodejs#43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: nodejs#43148
Fixes: nodejs#44415
PR-URL: nodejs#44423
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flarna Flarna Flarna approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@Qard Qard Qard approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. 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
8 participants
@targos @nodejs-github-bot @Qard @bnoordhuis @jasnell @cjihrig @gengjiawen @Flarna