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.

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

deps: V8: partially cherry-pick 8953e49478 #55274

Closed
wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Member

Very partial cherry-pick of upstream commit v8/v8@8953e49478 that fixes a hang that shows up in debug builds because of a buggy sanity check when computing relationships between command line flags.

Example of the hang:

#include "v8.h"
#include "libplatform/libplatform.h"
int main(void) {
  // works: v8::V8::SetFlagsFromString("--gc-global --noincremental-marking");
  v8::V8::SetFlagsFromString("--gc-global");
  v8::V8::SetFlagsFromString("--noincremental-marking");
  v8::V8::InitializePlatform(v8::platform::NewDefaultPlatform().release());
  v8::V8::Initialize(); // hangs in ComputeFlagListHash when defined(DEBUG)
}

NB: fix already exists in main; newer V8

Very partial cherry-pick of upstream commit v8/v8@8953e49478 that fixes
a hang that shows up in debug builds because of a buggy sanity check
when computing relationships between command line flags.

Example of the hang:

    #include "v8.h"
    #include "libplatform/libplatform.h"
    int main(void) {
      // works: v8::V8::SetFlagsFromString("--gc-global --noincremental-marking");
      v8::V8::SetFlagsFromString("--gc-global");
      v8::V8::SetFlagsFromString("--noincremental-marking");
      v8::V8::InitializePlatform(v8::platform::NewDefaultPlatform().release());
      v8::V8::Initialize(); // hangs in ComputeFlagListHash when defined(DEBUG)
    }
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch. v8 engine Issues and PRs related to the V8 dependency. labels Oct 5, 2024
@RedYetiDev RedYetiDev added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 5, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@bnoordhuis bnoordhuis added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Oct 7, 2024
@targos targos removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 7, 2024
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Oct 7, 2024

CleanShot 2024-10-07 at 08 40 51

Commit queue would have failed.

@bnoordhuis
Copy link
Member Author

That cannot be anything but a flake because this change only affects debug builds.

@targos
Copy link
Member

targos commented Oct 7, 2024

Yes, but automation cannot know that. Now it's green.

@targos targos added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 7, 2024
targos pushed a commit that referenced this pull request Oct 7, 2024
Very partial cherry-pick of upstream commit v8/v8@8953e49478 that fixes
a hang that shows up in debug builds because of a buggy sanity check
when computing relationships between command line flags.

Example of the hang:

    #include "v8.h"
    #include "libplatform/libplatform.h"
    int main(void) {
      // works: v8::V8::SetFlagsFromString("--gc-global --noincremental-marking");
      v8::V8::SetFlagsFromString("--gc-global");
      v8::V8::SetFlagsFromString("--noincremental-marking");
      v8::V8::InitializePlatform(v8::platform::NewDefaultPlatform().release());
      v8::V8::Initialize(); // hangs in ComputeFlagListHash when defined(DEBUG)
    }

PR-URL: #55274
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos
Copy link
Member

targos commented Oct 7, 2024

Commit queue doesn't work with release branches. Landed in 2f65b3f.

@targos targos closed this Oct 7, 2024
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. v8 engine Issues and PRs related to the V8 dependency. v22.x v22.x Issues that can be reproduced on v22.x or PRs targeting the v22.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants