napi: implement napi_is_detached_arraybuffer by lundibundi · Pull Request #30613 · nodejs/node · GitHub

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript
specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer.

Closes: #29955

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

Also superseeds #30317.

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: nodejs#29955

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

section numbers tend to change, i'd just say "as defined in the ecmascript language specifiation."

This looks like a good idea but a lot of the other functions (in this doc) have links to the appropriate sections in the spec so I'm not sure.

Wow, another ICE here

https://ci.nodejs.org/job/node-test-commit-arm/nodes=debian9-docker-armv7/27580/console
debian9-docker-armv7

23:37:07 ../src/api/exceptions.cc: In function 'v8::Local<v8::Value> node::UVException(v8::Isolate*, int, const char*, const char*, const char*, const char*)':
23:37:07 ../src/api/exceptions.cc:146:53: internal compiler error: Segmentation fault
23:37:07    e->Set(env->context(), env->code_string(), js_code).Check();
23:37:07                                                      ^
23:37:07 Please submit a full bug report,
23:37:07 with preprocessed source if appropriate.
23:37:07 See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.
23:37:07 libnode.target.mk:330: recipe for target '/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/api/exceptions.o' failed
23:37:07 make[2]: *** [/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/api/exceptions.o] Error 1

/cc @addaleax

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

Just found some nits in tests... 😅

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

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

Well, the second ICE on the same machine

https://ci.nodejs.org/job/node-test-commit-arm/27606/nodes=debian9-docker-armv7/console
debian9-docker-armv7

17:38:16 ../src/node_v8.cc: In function 'void node::Initialize(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, void*)':
17:38:16 ../src/node_v8.cc:175:57: internal compiler error: Segmentation fault
17:38:16                Uint32::NewFromUnsigned(env->isolate(), i)).Check();
17:38:16                                                          ^
17:38:16 ../src/node_v8.cc:51:3: note: in expansion of macro 'V'
17:38:16    V(2, total_physical_size, kTotalPhysicalSizeIndex)                          \
17:38:16    ^
17:38:16 ../src/node_v8.cc:177:3: note: in expansion of macro 'HEAP_STATISTICS_PROPERTIES'
17:38:16    HEAP_STATISTICS_PROPERTIES(V)
17:38:16    ^~~~~~~~~~~~~~~~~~~~~~~~~~
17:38:16 Please submit a full bug report,
17:38:16 with preprocessed source if appropriate.
17:38:16 See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.
17:38:16 libnode.target.mk:330: recipe for target '/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/node_v8.o' failed
17:38:16 make[2]: *** [/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/node_v8.o] Error 1
17:38:16 make[2]: *** Waiting for unfinished jobs....

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

Hm, ICE on debian9-docker-armv7 once again, not sure how to fix that.

22:57:02 ../src/node_v8.cc: In function 'void node::Initialize(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, void*)':
22:57:02 ../src/node_v8.cc:169:59: internal compiler error: Segmentation fault
22:57:02                                 env->heap_statistics_buffer(),
22:57:02                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
22:57:02 Please submit a full bug report,
22:57:02 with preprocessed source if appropriate.
22:57:02 See <file:///usr/share/doc/gcc-6/README.Bugs> for instructions.
22:57:02 libnode.target.mk:330: recipe for target '/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/node_v8.o' failed
22:57:02 make[2]: *** [/home/iojs/build/workspace/node-test-commit-arm/nodes/debian9-docker-armv7/out/Release/obj.target/libnode/src/node_v8.o] Error 1

/cc @nodejs/n-api @addaleax perhaps you can help?

@lundibundi Somewhere @bnoordhuis suggested that those ICEs might be the result of an out-of-memory situation … I think it’s a machine that doesn’t have a ton of memory, so that might make sense?

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

LGTM once my suggestion addition of the stability is added.

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

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: #29955 PR-URL: #30613 Fixes: #29955 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This needs a backport to land on v12.x-staging because V8 7.8 doesn't have ArrayBuffer::GetBackingStore

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: nodejs#29955 Backport-PR-URL: nodejs#31422 PR-URL: nodejs#30613 Fixes: nodejs#29955 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Refs: nodejs/node#29768 Refs: nodejs/node#30613

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: #29955 Backport-PR-URL: #31422 PR-URL: #30613 Fixes: #29955 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: nodejs#29955 PR-URL: nodejs#30613 Fixes: nodejs#29955 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: #29955 PR-URL: #30613 Backport-PR-URL: #33061 Fixes: #29955 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Refs: nodejs/node#29768 Refs: nodejs/node#30613

Refs: nodejs/node#29768 Refs: nodejs/node#30613 PR-URL: #659 Refs: nodejs/node#29768 Refs: nodejs/node#30613 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

Refs: nodejs/node#29768 Refs: nodejs/node#30613 PR-URL: nodejs#659 Refs: nodejs/node#29768 Refs: nodejs/node#30613 Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

n-api: implement napi_is_detached_arraybuffer

5cb8752

This implements ArrayBuffer#IsDetachedBuffer operation as per ECMAScript specification Section 24.1.1.2 https://tc39.es/ecma262/#sec-isdetachedbuffer Closes: nodejs#29955

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 23, 2019

lundibundi added the node-api Issues and PRs related to the Node-API. label Nov 23, 2019

addaleax approved these changes Nov 23, 2019

View reviewed changes

devsnek approved these changes Nov 23, 2019

View reviewed changes

devnexen approved these changes Nov 24, 2019

View reviewed changes

legendecas reviewed Nov 24, 2019

View reviewed changes

Comment thread test/js-native-api/test_typedarray/test_typedarray.c Outdated

Comment thread test/js-native-api/test_typedarray/test_typedarray.c Outdated

lundibundi added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 24, 2019

lundibundi removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Nov 24, 2019

fixup! n-api: implement napi_is_detached_arraybuffer

5bf270e

legendecas reviewed Nov 24, 2019

View reviewed changes

Comment thread src/js_native_api_v8.cc

fixup! n-api: implement napi_is_detached_arraybuffer

7352a88

legendecas approved these changes Nov 24, 2019

View reviewed changes

lundibundi mentioned this pull request Nov 27, 2019

node-test-commit-arm compiler ICE debian9-docker-armv7 nodejs/build#2066

Closed

mhdawson reviewed Nov 27, 2019

View reviewed changes

Comment thread doc/api/n-api.md

mhdawson approved these changes Nov 27, 2019

View reviewed changes

fixup! n-api: implement napi_is_detached_arraybuffer

8dc4a18

addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 28, 2019

BridgeAR mentioned this pull request Dec 3, 2019

v13.3.0 proposal #30774

Merged

targos added backport-blocked-v12.x and removed backport-blocked-v12.x labels Jan 14, 2020

lundibundi mentioned this pull request Jan 20, 2020

[v12.x backport] n-api: implement napi_is_detached_arraybuffer #31422

Closed

4 tasks

lundibundi added backport-open-v12.x and removed backport-requested-v12.x labels Jan 20, 2020

tniessen added a commit to tniessen/node-addon-api that referenced this pull request Jan 26, 2020

Add ArrayBuffer::Detach() and ::IsDetached()

023888b

Refs: nodejs/node#29768 Refs: nodejs/node#30613

tniessen mentioned this pull request Jan 26, 2020

Add ArrayBuffer::Detach() and ::IsDetached() nodejs/node-addon-api#659

Closed

tniessen added a commit to tniessen/node-addon-api that referenced this pull request Jan 26, 2020

Add ArrayBuffer::Detach() and ::IsDetached()

bcd03dd

Refs: nodejs/node#29768 Refs: nodejs/node#30613

MylesBorins added backported-to-v12.x and removed backport-open-v12.x labels Jan 30, 2020

targos mentioned this pull request Jan 30, 2020

v12.15.0 release proposal #31368

Closed

MylesBorins mentioned this pull request Feb 8, 2020

v12.16.0 proposal #31691

Merged

legendecas mentioned this pull request May 2, 2020

[v10.x] n-api: backport ArrayBuffer detaching APIs #33061

Closed

richardlau mentioned this pull request Jul 2, 2020

v10.22.0 proposal #34170

Merged

4 tasks

tniessen added a commit to tniessen/node-addon-api that referenced this pull request Nov 5, 2020

Add ArrayBuffer::Detach() and ::IsDetached()

ad9f5b0

Refs: nodejs/node#29768 Refs: nodejs/node#30613

+              The `ArrayBuffer` is considered detached if its internal data is `null`.
+              This API represents the invocation of the `ArrayBuffer` `IsDetachedBuffer`
+              operation as defined in [Section 24.1.1.2][] of the ECMAScript Language

napi: implement napi_is_detached_arraybuffer by lundibundi · Pull Request #30613 · nodejs/node · GitHub

Uh oh!

Uh oh!

napi: implement napi_is_detached_arraybuffer - #30613

napi: implement napi_is_detached_arraybuffer#30613lundibundi wants to merge 4 commits intonodejs:masternodejs/node:masterfrom lundibundi:impl-napi-is-detached-29955lundibundi/node:impl-napi-is-detached-29955Copy head branch name to clipboard

Conversation

lundibundi commented Nov 23, 2019

Checklist

Uh oh!

nodejs-github-bot commented Nov 23, 2019

Uh oh!

devsnek Nov 23, 2019

Choose a reason for hiding this comment

Uh oh!

lundibundi Nov 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lundibundi commented Nov 24, 2019

Uh oh!

nodejs-github-bot commented Nov 24, 2019

Uh oh!

Uh oh!

Uh oh!

legendecas commented Nov 24, 2019

Uh oh!

nodejs-github-bot commented Nov 24, 2019

Uh oh!

Uh oh!

nodejs-github-bot commented Nov 24, 2019

Uh oh!

lundibundi commented Nov 24, 2019

Uh oh!

nodejs-github-bot commented Nov 24, 2019

Uh oh!

lundibundi commented Nov 25, 2019

Uh oh!

addaleax commented Nov 26, 2019

Uh oh!

nodejs-github-bot commented Nov 27, 2019

Uh oh!

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Nov 28, 2019

Uh oh!

targos commented Jan 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

napi: implement napi_is_detached_arraybuffer#30613
lundibundi wants to merge 4 commits into
nodejs:masterfrom
lundibundi:impl-napi-is-detached-29955

lundibundi Nov 24, 2019 •
edited
Loading