napi: implement napi_is_detached_arraybuffer #30613

lundibundi · 2019-11-23T21:25:03Z

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

nodejs-github-bot · 2019-11-23T21:28:55Z

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

devsnek · 2019-11-23T21:41:25Z

doc/api/n-api.md

+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


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.

lundibundi · 2019-11-24T13:16:30Z

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

nodejs-github-bot · 2019-11-24T13:16:48Z

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

test/js-native-api/test_typedarray/test_typedarray.c

legendecas · 2019-11-24T13:18:22Z

Just found some nits in tests... 😅

nodejs-github-bot · 2019-11-24T13:50:15Z

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

src/js_native_api_v8.cc

nodejs-github-bot · 2019-11-24T14:27:37Z

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

lundibundi · 2019-11-24T20:48:40Z

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....

nodejs-github-bot · 2019-11-24T20:49:17Z

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

lundibundi · 2019-11-25T15:37:27Z

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?

addaleax · 2019-11-26T13:49:09Z

@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?

nodejs-github-bot · 2019-11-27T17:29:00Z

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

doc/api/n-api.md

mhdawson

LGTM once my suggestion addition of the stability is added.

nodejs-github-bot · 2019-11-28T06:47:14Z

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

targos · 2020-01-14T10:56:27Z

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>

Refs: nodejs/node#29768 Refs: nodejs/node#30613 PR-URL: nodejs/node-addon-api#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

test/js-native-api/test_typedarray/test_typedarray.c Outdated Show resolved Hide resolved

test/js-native-api/test_typedarray/test_typedarray.c Outdated Show resolved Hide resolved

lundibundi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 24, 2019

lundibundi removed the author ready PRs that have at least one approval, no pending requests for changes, 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

src/js_native_api_v8.cc Show resolved Hide resolved

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

doc/api/n-api.md Show resolved Hide resolved

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

targos added backport-requested-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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

napi: implement napi_is_detached_arraybuffer #30613

napi: implement napi_is_detached_arraybuffer #30613

lundibundi commented Nov 23, 2019

nodejs-github-bot commented Nov 23, 2019

devsnek Nov 23, 2019

lundibundi Nov 24, 2019 •

edited

lundibundi commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

legendecas commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

lundibundi commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

lundibundi commented Nov 25, 2019

addaleax commented Nov 26, 2019

nodejs-github-bot commented Nov 27, 2019

mhdawson left a comment

nodejs-github-bot commented Nov 28, 2019

targos commented Jan 14, 2020

napi: implement napi_is_detached_arraybuffer #30613

napi: implement napi_is_detached_arraybuffer #30613

Conversation

lundibundi commented Nov 23, 2019

Checklist

nodejs-github-bot commented Nov 23, 2019

devsnek Nov 23, 2019

Choose a reason for hiding this comment

lundibundi Nov 24, 2019 • edited

Choose a reason for hiding this comment

lundibundi commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

legendecas commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

lundibundi commented Nov 24, 2019

nodejs-github-bot commented Nov 24, 2019

lundibundi commented Nov 25, 2019

addaleax commented Nov 26, 2019

nodejs-github-bot commented Nov 27, 2019

mhdawson left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Nov 28, 2019

targos commented Jan 14, 2020

lundibundi Nov 24, 2019 •

edited