src: migrated to new V8 ArrayBuffer API by thangktran · Pull Request #30782

src: migrated to new V8 ArrayBuffer API by thangktran · Pull Request #30782 · nodejs/node

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 added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Dec 6, 2019

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 added a commit to addaleax/node that referenced this pull request

Dec 13, 2019

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

gabrielschulhof pushed a commit that referenced this pull request

Dec 14, 2019

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 <apapirovski@mac.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

addaleax added a commit to addaleax/node that referenced this pull request

Feb 5, 2020

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

addaleax added a commit that referenced this pull request

Feb 8, 2020

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 <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>