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

[v10.x] deps: cherry-pick 88f8fe1 from upstream V8 #27894

Closed

Conversation

addaleax
Copy link
Member

Original commit message:

Fix collection iterator preview with deleted entries

We used to assume that we know the remaining entries returned by the
iterator based on the current index. However, that is not accurate,
since entries skipped by the current index could be deleted.

In the new approach, we allocate conservatively and shrink the result.

R=neis@chromium.org

Bug: v8:8433
Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
Reviewed-on: https://chromium-review.googlesource.com/c/1325966
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
Handle constructor and the Shrink() method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: #27882
PR-URL: #24514
Refs: #24053

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
`Handle` constructor and the `Shrink()` method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: nodejs#27882
PR-URL: nodejs#24514
Refs: nodejs#24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@addaleax addaleax added v10.x v8 engine Issues and PRs related to the V8 dependency. labels May 26, 2019
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 26, 2019

@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member Author

I’m fairly certain the V8 CI failure is unrelated, but just in case, here’s another try: https://ci.nodejs.org/job/node-test-commit-v8-linux/2322/

/cc @nodejs/v8

@addaleax
Copy link
Member Author

V8 CI for v10.x-staging, to make sure it really was broken before: https://ci.nodejs.org/job/node-test-commit-v8-linux/2323/

@addaleax
Copy link
Member Author

The V8 CI for v10.x-staging failed the same way, so I think this is ready for merging.

BethGriggs pushed a commit that referenced this pull request Jun 6, 2019
Original commit message:

    Fix collection iterator preview with deleted entries

    We used to assume that we know the remaining entries returned by the
    iterator based on the current index. However, that is not accurate,
    since entries skipped by the current index could be deleted.

    In the new approach, we allocate conservatively and shrink the result.

    R=neis@chromium.org

    Bug: v8:8433
    Change-Id: I38a3004dc3af292daabb454bb76f38d65ef437e8
    Reviewed-on: https://chromium-review.googlesource.com/c/1325966
    Commit-Queue: Yang Guo <yangguo@chromium.org>
    Reviewed-by: Georg Neis <neis@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#57360}

[The backport to v10.x resolves merge conflicts due to a different way
of accessing the “hole” value in V8, different signatures of the
`Handle` constructor and the `Shrink()` method, and neighbouring-line
conflicts in the test file.]

Refs: v8/v8@88f8fe1
Fixes: #27882

Backport-PR-URL: #27894
PR-URL: #24514
Refs: #24053
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@BethGriggs
Copy link
Member

Landed on v10.x-staging

@BethGriggs BethGriggs closed this Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants