src: migrated to new V8 ArrayBuffer API #30782

thangktran · 2019-12-03T22:24:00Z

ArrayBuffer without BackingStore will soon be deprecated.

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

Trott · 2019-12-04T00:04:44Z

Micro-nit for OP or whoever lands this: s/migrated/migrate/ in the commit message first line. (We're not so great at enforcing it, but our commit guidelines say to make the first word after the subsystem an imperative verb--basically, present tense.)

addaleax

Largely LGTM, nice work!

src/aliased_buffer.h

src/js_native_api_v8.cc

src/node_buffer.cc

addaleax · 2019-12-04T00:24:22Z

src/env-inl.h

-                              buf.base,
-                              buf.len,
-                              v8::ArrayBufferCreationMode::kInternalized);
+                              std::move(backing));


So… This looks correct and I currently don’t have any better ideas either, but maybe to give an idea about the “big picture” here:

The idea behind AllocatedBuffer is to have something that gets turned into an ArrayBuffer in the most direct way possible. In that sense, it is basically our own version of BackingStore – a wrapper class for memory tracked through the current ArrayBuffer::Allocator. A “nicer” way to fix this would be to e.g. store the data as the std::shared_ptr<BackingStore> directly instead of using a uv_buf_t. However, I think that doesn’t work currently, because we’d have no way to implement e.g. .release() for that kind of scenario.

It might be nice if we had a way to avoid free callbacks that only run v8::ArrayBuffer::Allocator::Free() like this one. That would require an V8 API change, though, and I’m not sure what the best way for that would be.

Or, alternatively, it would be nice if we could implement AllocatedBuffer in terms of std::shared_ptr<BackingStore> as mentioned above; in that case, we’d have to figure out something clever for AllocatedBuffer::release().

(To be clear, this isn’t a request to change the code here. It’s just me rambling and trying to give some context.)

I'll think of something and prepare a PR for this.

src/js_native_api_v8.cc

src/node_buffer.cc

test/addons/worker-buffer-callback/binding.cc

thangktran · 2019-12-04T18:34:58Z

Micro-nit for OP or whoever lands this: s/migrated/migrate/ in the commit message first line. (We're not so great at enforcing it, but our commit guidelines say to make the first word after the subsystem an imperative verb--basically, present tense.)

fixed

nodejs-github-bot · 2019-12-04T19:11:50Z

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

joaocgreis · 2019-12-05T03:09:07Z

Windows CI re-run: https://ci.nodejs.org/job/node-test-commit-windows-fanned/31994/

nodejs-github-bot · 2019-12-05T05:34:35Z

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

Trott

This fails make lint-js. Can you fix it up?

Trott

Oh, wait, the linter issues are probably a different thing that got fixed already. Sorry for the noise.

nodejs-github-bot · 2019-12-06T00:49:34Z

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

nodejs-github-bot · 2019-12-06T01:04:34Z

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

Trott · 2019-12-06T02:44:42Z

Does the number of crashing tests in CI with this change seems to suggest there's a problem with it that hasn't been identified yet?

addaleax · 2019-12-06T03:19:19Z

@Trott Yes, definitely. The failing check is here:

node/deps/v8/src/objects/backing-store.cc

Line 581 in 418dd68

CHECK(result.second);

It’s odd that it’s failing inconsistently across multiple tests, though.

src/node_worker.cc

addaleax · 2019-12-06T03:29:05Z

My guess would be that the cause is that Http2Session instances may be allocated in the same location as previous instances of that class, before the ArrayBuffer objects associated with those previous instances are garbage collected.

In that case the previous BackingStores would also still exist and refer to now-deallocated memory, and V8 refuses to create BackingStores for the new ones. That’s actually a fair complaint by V8, in a way.

I think a possible solution would be to call ArrayBuffer::Detach() from the Http2Session destructor. That should also keep us from accidentally introducing crashes if we happen to mess up in JS land.

codecov-io · 2019-12-06T04:17:03Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d7b8ae7). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #30782   +/-   ##
=========================================
  Coverage          ?   97.28%           
=========================================
  Files             ?      187           
  Lines             ?    62462           
  Branches          ?        0           
=========================================
  Hits              ?    60767           
  Misses            ?     1695           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7b8ae7...850a80b. Read the comment docs.

thangktran · 2019-12-06T13:49:34Z

ping for review and CI.

targos · 2019-12-06T13:52:17Z

Do the changes in test/js-native-api mean that there is a beraking change for N-API ?

addaleax · 2019-12-06T13:53:39Z

src/node_http2.cc

+            ->Externalize(array_buffer_to_detach->GetBackingStore());
+      array_buffer_to_detach->Detach();
+    }
+  }


Uh … I think detaching the stream_buf_ab_ would invalidate stream chunks emitted to JS? Can you explain these changes here?

sure. so stream_buf_ab_ is the only ArrayBuffer from Http2Session/node_http2.cc so i believe the error has to do with it. So i want to make sure that the ArrayBuffer under stream_buf_ab_ is detached right before Reset() is called. I ran all tests and they all passed, did i miss something?

so stream_buf_ab_ is the only ArrayBuffer from Http2Session/node_http2.cc so i believe the error has to do with it.

There’s the one that’s being changed here above, too (the one for js_fields_). I think that’s the problematic one, because we allocate the Http2Session ourselves but don’t invalidate/detach the ArrayBuffer when the underlying Http2Session memory is freed.

I think that means that it would need to be stored on the Http2Session in a v8::Global, yes.

I ran all tests and they all passed, did i miss something?

Hm, that’s surprising … maybe our tests just don’t verify this :/ I’ll see if I can come up with something for that…

I ran all tests and they all passed, did i miss something?

Hm, that’s surprising … maybe our tests just don’t verify this :/ I’ll see if I can come up with something for that…

I did some digging here and it turns out this was still working because stream_buf_ab_ is never weak, so ClearWeak() always returned nullptr, and the array buffer never actually got detached.

That means js_fields_ is our main suspect here? I'm working on a fix for this.

Yes, exactly – I haven’t verified that it’s the thing that caused the crashes earlier in CI, but it is something that could cause crashes.

Can you remove the block of changes here? As mentioned above, it doesn’t do anything right now :)

addaleax · 2019-12-06T13:55:07Z

Do the changes in test/js-native-api mean that there is a beraking change for N-API ?

Hm … I guess this is a breaking change for addons that return an ArrayBuffer for static data, yes.

Trott · 2019-12-06T19:01:38Z

@nodejs/n-api

BridgeAR · 2019-12-13T06:22:37Z

The CI currently has lots of crashes related to workers. I guess that it might be related to this PR.

Example failure: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/343/RUN_SUBSET=3,nodes=win2012r2-COMPILED_BY-vs2019-x86/testReport/junit/(root)/test/parallel_test_worker_error_stack_getter_throws/

addaleax · 2019-12-13T15:19:32Z

@BridgeAR @nodejs/build Core dumps might be massively helpful here, once again :)

I’ll take a look and see if I can reproduce something and/or see if there was anything that we missed.

This fixes flaky tests that crashed because the allocations ended up at positions of previously allocated `ArrayBuffer`s that were still in the backing store table. In particular, there was a race condition window between destroying a Worker thread’s `Environment` and destroying its `Isolate` in which the underlying memory was already released but the `ArrayBuffer` was still existent, meaning that new memory could be allocated at the address of the previous `ArrayBuffer`. Refs: nodejs#30782

addaleax · 2019-12-13T20:20:36Z

@BridgeAR @thangktran #30946 should address those failures, please take a look :)

addaleax · 2019-12-13T20:30:03Z

src/env-inl.h

+    CHECK_NOT_NULL(deleter_data);
+
+    static_cast<v8::ArrayBuffer::Allocator*>(deleter_data)
+        ->Free(data, length);


There’s an issue with this that I didn’t realize during review, sorry … see https://bugs.chromium.org/p/v8/issues/detail?id=9908#c10 for details

@addaleax should i force-push here or open a new PR?

@thangktran I think neither. I believe @addaleax already opened a PR to fix: #30946

@Trott i checked it just now and it's actually still there.

@Trott This here is a V8 API issue, #30946 fixes a different bug in our code. So there is something we need to do.

I think the 3 options here are:

Temporarily reverting the changes that add the v8::ArrayBuffer::Allocator::Free calls and go back to using the deprecated APIs, effectively blocking the upgrade to 8.0 or 8.1 (not sure which)

Modifying our embedder API to let embedders and ourselves pass a std::shared_ptr<v8::ArrayBuffer::Allocator> that is also passed to V8 and then use a pointer to that instead of a raw v8::ArrayBuffer::Allocator* (I don’t like this one, it’s quite a bit of work and complexity with little gain)

Implementing the V8 API suggestion in the V8 bug linked above and then using that. I think that would be the ideal one, but let’s wait for @ulan’s response in the issue first. @thangktran If this works out and you haven’t contributed to V8 before, I’m happy to help answer any questions – I know their process can be a bit unusual for those who aren’t used to it :)

@addaleax : Thank you for offering me help. I'm looking forward to it :D
What is your opinion about @ulan 's alternatives? :

Before we go ahead with the new constructor, I want to discuss the feasibility of the third option:

Change AllocatedBuffer [1] to allocate and keep std::unique_ptrv8::BackingStore.

Add Reallocate method to ArrayBufferAllocator (the default implementation would simply allocate a new buffer).

Add a static Reallocate method to v8::BackingStore that takes a unique_ptr and returns a unique_ptr. Internally it calls Reallocate of the array buffer allocator.

@thangktran Thanks for the ping! I’ve commented in the issue. Basically, I’m good with Ulan’s suggestion. 👍

This fixes flaky tests that crashed because the allocations ended up at positions of previously allocated `ArrayBuffer`s that were still in the backing store table. In particular, there was a race condition window between destroying a Worker thread’s `Environment` and destroying its `Isolate` in which the underlying memory was already released but the `ArrayBuffer` was still existent, meaning that new memory could be allocated at the address of the previous `ArrayBuffer`. Refs: #30782 PR-URL: #30946 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>

It looks like it’s virtually impossible at this point to create “fake” backing stores for objects that don’t fully own their memory allocations, like the sub-field `js_fields_` of `Http2Session`. In particular, it turns out that an `ArrayBuffer` cannot always be easily separated from its backing store in that situation through by detaching it. This commit gives the JS-exposed parts of the class its own memory allocation and its own backing store, simplifying the code a bit and fixing flakiness coming from it, at the cost of one additional layer of indirection when accessing the data. Refs: nodejs#30782 Fixes: nodejs#31107

It looks like it’s virtually impossible at this point to create “fake” backing stores for objects that don’t fully own their memory allocations, like the sub-field `js_fields_` of `Http2Session`. In particular, it turns out that an `ArrayBuffer` cannot always be easily separated from its backing store in that situation through by detaching it. This commit gives the JS-exposed parts of the class its own memory allocation and its own backing store, simplifying the code a bit and fixing flakiness coming from it, at the cost of one additional layer of indirection when accessing the data. Refs: #30782 Fixes: #31107 PR-URL: #31648 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>

- (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL (James M Snell) [#31166](#31166) - (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection (James M Snell) [#28396](#28396) - (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL (James M Snell) [#31164](#31164) - (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL (James M Snell) [#31167](#31167) - (SEMVER-MAJOR) os: move tmpDir() to EOL (James M Snell)[#31169](#31169) - (SEMVER-MAJOR) src: remove deprecated wasm type check (Clemens Backes) [#32116](#32116) - (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL (James M Snell) [#31165](#31165) - (SEMVER-MINOR) doc: deprecate process.mainModule (Antoine du HAMEL) [#32232](#32232) - (SEMVER-MINOR) doc: deprecate process.umask() with no arguments (Colin Ihrig) [#32499](#32499) - src: migrate to new V8 ArrayBuffer API (Thang Tran) [#30782](#30782) It is possible that this change will impact some native addons using `ArrayBuffer`. - (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x (AshCripps)[#32454](#32454) - (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: remove SmartOS from official binaries (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) win: block running on EOL Windows versions (João Reis) [#31954](#31954) It is expected that there will be an ABI mismatch on ARM between the Node.js binary and native addons. - [#30786](#30786) - (SEMVER-MAJOR) deps: update V8 to 8.1.307.20 (Matheus Marchini) [#32116](#32116) - cli, report: move --report-on-fatalerror to stable (Colin Ihrig) [#32496](#32496) - deps: upgrade to libuv 1.37.0 (Colin Ihrig) [#32866](#32866) - fs: add fs/promises alias module (Gus Caplan) [#31553](#31553) - module: remove experimental modules warning (Guy Bedford) [#31974](#31974) PR-URL: #32181

Deprecations: - (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL (James M Snell) [#31166](#31166) - (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection (James M Snell) [#28396](#28396) - (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL (James M Snell) [#31164](#31164) - (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL (James M Snell) [#31167](#31167) - (SEMVER-MAJOR) os: move tmpDir() to EOL (James M Snell)[#31169](#31169) - (SEMVER-MAJOR) src: remove deprecated wasm type check (Clemens Backes) [#32116](#32116) - (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL (James M Snell) [#31165](#31165) - (SEMVER-MINOR) doc: deprecate process.mainModule (Antoine du HAMEL) [#32232](#32232) - (SEMVER-MINOR) doc: deprecate process.umask() with no arguments (Colin Ihrig) [#32499](#32499) ECMAScript Modules - Experimental Warning Removal: - module: remove experimental modules warning (Guy Bedford) [#31974](#31974) In Node.js 13 we removed the need to include the --experimental-modules flag, but when running EcmaScript Modules in Node.js, this would still result in a warning ExperimentalWarning: The ESM module loader is experimental. As of Node.js 14 there is no longer this warning when using ESM in Node.js. However, the ESM implementation in Node.js remains experimental. As per our stability index: “The feature is not subject to Semantic Versioning rules. Non-backward compatible changes or removal may occur in any future release.” Users should be cautious when using the feature in production environments. Please keep in mind that the implementation of ESM in Node.js differs from the developer experience you might be familiar with. Most transpilation workflows support features such as optional file extensions or JSON modules that the Node.js ESM implementation does not support. It is highly likely that modules from transpiled environments will require a certain degree of refactoring to work in Node.js. It is worth mentioning that many of our design decisions were made with two primary goals. Spec compliance and Web Compatibility. It is our belief that the current implementation offers a future proof model to authoring ESM modules that paves the path to Universal JavaScript. Please read more in our documentation. The ESM implementation in Node.js is still experimental but we do believe that we are getting very close to being able to call ESM in Node.js “stable”. Removing the warning is a huge step in that direction. Migrate to new V8 ArrayBuffer API: - src: migrate to new V8 ArrayBuffer API (Thang Tran) [#30782](#30782) It is possible that this change will impact some native addons using `ArrayBuffer`. Toolchain and Compiler Upgrades: - (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x (AshCripps)[#32454](#32454) - (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: remove SmartOS from official binaries (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) win: block running on EOL Windows versions (João Reis) [#31954](#31954) It is expected that there will be an ABI mismatch on ARM between the Node.js binary and native addons. Native addons are only broken if they interact with `std::shared_ptr`. This is expected to be fixed in a later version of Node.js 14. - [#30786](#30786) Update to V8 8.1: - (SEMVER-MAJOR) deps: update V8 to 8.1.307.20 (Matheus Marchini) [#32116](#32116) Other Notable Changes: - cli, report: move --report-on-fatalerror to stable (Colin Ihrig) [#32496](#32496) - deps: upgrade to libuv 1.37.0 (Colin Ihrig) [#32866](#32866) - fs: add fs/promises alias module (Gus Caplan) [#31553](#31553) PR-URL: #32181

Deprecations: - (SEMVER-MAJOR) crypto: move pbkdf2 without digest to EOL (James M Snell) [#31166](#31166) - (SEMVER-MAJOR) fs: deprecate closing FileHandle on garbage collection (James M Snell) [#28396](#28396) - (SEMVER-MAJOR) http: move OutboundMessage.prototype.flush to EOL (James M Snell) [#31164](#31164) - (SEMVER-MAJOR) lib: move GLOBAL and root aliases to EOL (James M Snell) [#31167](#31167) - (SEMVER-MAJOR) os: move tmpDir() to EOL (James M Snell)[#31169](#31169) - (SEMVER-MAJOR) src: remove deprecated wasm type check (Clemens Backes) [#32116](#32116) - (SEMVER-MAJOR) stream: move \_writableState.buffer to EOL (James M Snell) [#31165](#31165) - (SEMVER-MINOR) doc: deprecate process.mainModule (Antoine du HAMEL) [#32232](#32232) - (SEMVER-MINOR) doc: deprecate process.umask() with no arguments (Colin Ihrig) [#32499](#32499) ECMAScript Modules - Experimental Warning Removal: - module: remove experimental modules warning (Guy Bedford) [#31974](#31974) In Node.js 13 we removed the need to include the --experimental-modules flag, but when running EcmaScript Modules in Node.js, this would still result in a warning ExperimentalWarning: The ESM module loader is experimental. As of Node.js 14 there is no longer this warning when using ESM in Node.js. However, the ESM implementation in Node.js remains experimental. As per our stability index: “The feature is not subject to Semantic Versioning rules. Non-backward compatible changes or removal may occur in any future release.” Users should be cautious when using the feature in production environments. Please keep in mind that the implementation of ESM in Node.js differs from the developer experience you might be familiar with. Most transpilation workflows support features such as optional file extensions or JSON modules that the Node.js ESM implementation does not support. It is highly likely that modules from transpiled environments will require a certain degree of refactoring to work in Node.js. It is worth mentioning that many of our design decisions were made with two primary goals. Spec compliance and Web Compatibility. It is our belief that the current implementation offers a future proof model to authoring ESM modules that paves the path to Universal JavaScript. Please read more in our documentation. The ESM implementation in Node.js is still experimental but we do believe that we are getting very close to being able to call ESM in Node.js “stable”. Removing the warning is a huge step in that direction. New V8 ArrayBuffer API: * **src**: migrate to new V8 ArrayBuffer API (Thang Tran) [#30782](#30782) Multiple ArrayBuffers pointing to the same base address are no longer allowed by V8. This may impact native addons. Toolchain and Compiler Upgrades: - (SEMVER-MAJOR) build: update macos deployment target to 10.13 for 14.x (AshCripps)[#32454](#32454) - (SEMVER-MAJOR) doc: update cross compiler machine for Linux armv7 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: update Centos/RHEL releases use devtoolset-8 (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) doc: remove SmartOS from official binaries (Richard Lau) [#32812](#32812) - (SEMVER-MAJOR) win: block running on EOL Windows versions (João Reis) [#31954](#31954) It is expected that there will be an ABI mismatch on ARM between the Node.js binary and native addons. Native addons are only broken if they interact with `std::shared_ptr`. This is expected to be fixed in a later version of Node.js 14. - [#30786](#30786) Update to V8 8.1: - (SEMVER-MAJOR) deps: update V8 to 8.1.307.20 (Matheus Marchini) [#32116](#32116) Other Notable Changes: - cli, report: move --report-on-fatalerror to stable (Colin Ihrig) [#32496](#32496) - deps: upgrade to libuv 1.37.0 (Colin Ihrig) [#32866](#32866) - fs: add fs/promises alias module (Gus Caplan) [#31553](#31553) PR-URL: #32181

Implement support for Electron 12. Major changes: - [node #30782](nodejs/node#30782) New ArrayBuffer backing store prevents reusing the same data pointer - [electron #27949](electron/electron#27949) Context isolation is turned on by default and prevents `coreclr-hosting` from working properly - [electron #18397](electron/electron#18397) & [electron #454](maximegris/angular-electron#454) lead to `app.allowRendererProcessReuse` being fixed to true on Electron 12. This required making `coreclr-hosting` context aware and implement proper re-use of the hosted CLR. Known Issues: - After reloading a renderer app the task scheduler sometimes hangs up, as the the `ThreadSafeFunction` does not get invoked. Code cleanup and small memory management improvements.

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Dec 3, 2019

addaleax approved these changes Dec 4, 2019

View reviewed changes

thangktran force-pushed the thangktran/issues/30529 branch 2 times, most recently from c7ae8d6 to 759fcdd Compare December 4, 2019 18:34

addaleax approved these changes Dec 4, 2019

View reviewed changes

Trott approved these changes Dec 6, 2019

View reviewed changes

Trott requested changes Dec 6, 2019

View reviewed changes

Trott approved these changes Dec 6, 2019

View reviewed changes

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

addaleax reviewed Dec 6, 2019

View reviewed changes

src/node_worker.cc Outdated Show resolved Hide resolved

thangktran force-pushed the thangktran/issues/30529 branch from 759fcdd to 52985d9 Compare December 6, 2019 13:47

addaleax reviewed Dec 6, 2019

View reviewed changes

Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 6, 2019

Trott added the semver-major PRs that contain breaking changes and should be released in the next major version. label Dec 6, 2019

addaleax closed this Dec 12, 2019

addaleax mentioned this pull request Dec 13, 2019

src: explicitly allocate backing stores for v8 stat buffers #30946

Closed

2 tasks

addaleax reviewed Dec 13, 2019

View reviewed changes

thangktran mentioned this pull request Jan 4, 2020

src: set arraybuffer_untransferable_private_symbol #31053

Closed

4 tasks

addaleax mentioned this pull request Feb 5, 2020

src: give Http2Session JS fields their own backing store #31648

Closed

2 tasks

BethGriggs mentioned this pull request Mar 10, 2020

2020-04-21, Version 14.0.0 (Current) #32181

Merged

nsabovic mentioned this pull request Apr 26, 2020

ffi-napi 2.5.0 crashes on Node 14 on macOS 10.15.4 node-ffi-napi/node-ffi-napi#71

Closed

lovell mentioned this pull request May 6, 2020

"Check failed: result.second" in Node 14.1 via Sharp 0.24 lovell/sharp#2196

Closed

strigona-worksight mentioned this pull request May 6, 2020

Node 14 ArrayBuffer changes causes grpc to crash. grpc/grpc-node#1407

Closed

joyeecheung mentioned this pull request Jun 8, 2020

src: retrieve binding data from the context #33139

Closed

3 tasks

sanosdole mentioned this pull request Mar 30, 2021

electron-12 sanosdole/nodeclrhost#50

Merged

src: migrated to new V8 ArrayBuffer API #30782

src: migrated to new V8 ArrayBuffer API #30782

Conversation

thangktran commented Dec 3, 2019

Checklist

Trott commented Dec 4, 2019

addaleax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thangktran commented Dec 4, 2019

nodejs-github-bot commented Dec 4, 2019

joaocgreis commented Dec 5, 2019

nodejs-github-bot commented Dec 5, 2019

Trott left a comment

Choose a reason for hiding this comment

Trott left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Dec 6, 2019

nodejs-github-bot commented Dec 6, 2019

Trott commented Dec 6, 2019

addaleax commented Dec 6, 2019

addaleax commented Dec 6, 2019

codecov-io commented Dec 6, 2019 • edited Loading

Codecov Report

thangktran commented Dec 6, 2019

targos commented Dec 6, 2019

Choose a reason for hiding this comment

thangktran Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Dec 6, 2019

Trott commented Dec 6, 2019

BridgeAR commented Dec 13, 2019

addaleax commented Dec 13, 2019

addaleax commented Dec 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax Dec 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Dec 6, 2019 •

edited

Loading

thangktran Dec 6, 2019 •

edited

Loading

addaleax Dec 6, 2019 •

edited

Loading

addaleax Dec 13, 2019 •

edited

Loading