FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer,n-api: fix double ArrayBuffer::Detach() during cleanup by addaleax · Pull Request #33039 · nodejs/node · GitHub

/ node Public

buffer,n-api: fix double ArrayBuffer::Detach() during cleanup - #33039

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:fix-napi-double-detach
ConversationCommits (1)ChecksFiles changed
Closed

buffer,n-api: fix double ArrayBuffer::Detach() during cleanup#33039
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:fix-napi-double-detach

Conversation

addaleax commented Apr 24, 2020

Copy link
Copy Markdown
Member

These calls could fail if the ArrayBuffer had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
5a88437 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: nodejs#33022
Refs: nodejs#30551
nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Apr 24, 2020
addaleax mentioned this pull request Apr 24, 2020
Closed
addaleax added lts-watch-v12.x node-api Issues and PRs related to the Node-API. labels Apr 24, 2020

nodejs-github-bot commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

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

targos commented Apr 24, 2020

Copy link
Copy Markdown
Member

Do we want to include this in v12.16.3?

addaleax commented Apr 24, 2020

Copy link
Copy Markdown
Member Author

@targos Sure, I’m okay with that. As you can guess, this is a really low-risk change.

nodejs-github-bot commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

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

mhdawson approved these changes Apr 24, 2020
View reviewed changes

mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

nodejs-github-bot commented Apr 24, 2020 •
edited by addaleax
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30961/ (:heavy_check_mark:)

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Apr 24, 2020
targos pushed a commit that referenced this pull request Apr 27, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
36993c0 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>

targos commented Apr 27, 2020

Copy link
Copy Markdown
Member

Landed in 36993c0

targos closed this Apr 27, 2020
targos removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. lts-watch-v12.x labels Apr 27, 2020
targos pushed a commit that referenced this pull request Apr 27, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
39a3cc1 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
targos mentioned this pull request Apr 27, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
8cde2f9 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
BethGriggs mentioned this pull request Apr 27, 2020
Merged
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
9bcfc8e 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
18fd841 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
BridgeAR mentioned this pull request Apr 28, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
1af08e1 
These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
legendecas legendecas approved these changes
juanarbol juanarbol approved these changes
Flarna Flarna approved these changes
+2 more reviewers
devnexen devnexen approved these changes
mhdawson mhdawson approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

napi_detach_arraybuffer aborts in Node 12.16.2 with an external arraybuffer

9 participants

Back | FazBrowse Home | New Git URL