(vee-eight-4.9) Replace more deprecated V8 API usage by ofrobots · Pull Request #5204 · nodejs/node · GitHub

Continuing from #5159, this PR replaces deprecated uses of SetAccessor and most uses of SetWeak on the vee-eight-4.9 branch.

The use of SetWeak in src/node_buffer.cc is more involved because it expects to get a handle to the weak object in the weak callback. I need to understand the buffer code a bit more before attempting to change that.

EDIT: the original change for SetWeak usage in node_contextify was flawed. I have removed that.

Remaining work:

SetWeak – src/node_contextify.cc: (vee-eight-4.9) contextify: update deprecated SetWeak usage #5392
SetWeak – src/node_buffer.cc
NewExternal — src/string_bytes.cc

See also: #4869
R=@bnoordhuis , @targos
/cc @nodejs/v8

Dynamic checks that CallbackInfo holds an ArrayBuffer handle can be converted into compiler enforced checks. Removed unused code, and other minor cleanup.

It would be nicer to make the field index a class constant.

Can we maybe make a comment in the header that the first internal field is reserved by node? Since switching to using array buffer I've been writing experimental modules that also make use of these fields.

Done. In the light of wanting a comment in the header, I chose to make it a header constant rather than class constant. 8be49858fab.

LGTM with a suggestion.

LGTM

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

Updating the use of SetWeak in node_contextify.cc might be a non-trivial amount of work. Let's do it in a follow-on PR. I haven't looked at NewExternal in string_bytes.cc yet.

Old style SetWeak is now deprecated, and weakness now works like phantom references. This means we no longer have a reference to the object in the weak callback. We use a kInternalFields style weak callback which provides us with the contents of 2 internal fields where we can squirrel away the native buffer pointer. We can no longer neuter the buffer in the weak callback, but that should be unnecessary as the object is going to be GC'd during the current gc cycle.

Second CI (also includes follow-on #5392): https://ci.nodejs.org/job/node-test-pull-request/1749/

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Dynamic checks that CallbackInfo holds an ArrayBuffer handle can be converted into compiler enforced checks. Removed unused code, and other minor cleanup. PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Old style SetWeak is now deprecated, and weakness now works like phantom references. This means we no longer have a reference to the object in the weak callback. We use a kInternalFields style weak callback which provides us with the contents of 2 internal fields where we can squirrel away the native buffer pointer. We can no longer neuter the buffer in the weak callback, but that should be unnecessary as the object is going to be GC'd during the current gc cycle. PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Landed as d47a982...7a863af.

PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Dynamic checks that CallbackInfo holds an ArrayBuffer handle can be converted into compiler enforced checks. Removed unused code, and other minor cleanup. PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Old style SetWeak is now deprecated, and weakness now works like phantom references. This means we no longer have a reference to the object in the weak callback. We use a kInternalFields style weak callback which provides us with the contents of 2 internal fields where we can squirrel away the native buffer pointer. We can no longer neuter the buffer in the weak callback, but that should be unnecessary as the object is going to be GC'd during the current gc cycle. PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Dynamic checks that CallbackInfo holds an ArrayBuffer handle can be converted into compiler enforced checks. Removed unused code, and other minor cleanup. PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

Old style SetWeak is now deprecated, and weakness now works like phantom references. This means we no longer have a reference to the object in the weak callback. We use a kInternalFields style weak callback which provides us with the contents of 2 internal fields where we can squirrel away the native buffer pointer. We can no longer neuter the buffer in the weak callback, but that should be unnecessary as the object is going to be GC'd during the current gc cycle. PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

This call was introduced in 827ee49 to avoid a crash in a later `Neuter()` call that has later been removed in ebbbc5a, rendering the original call unnecessary. Refs: nodejs#3624 Refs: nodejs#5204

This call was introduced in 827ee49 to avoid a crash in a later `Neuter()` call that has later been removed in ebbbc5a, rendering the original call unnecessary. Refs: #3624 Refs: #5204 PR-URL: #25479 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

This call was introduced in 827ee49 to avoid a crash in a later `Neuter()` call that has later been removed in ebbbc5a, rendering the original call unnecessary. Refs: nodejs/node#3624 Refs: nodejs/node#5204 PR-URL: nodejs/node#25479 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>

ofrobots added the v8 engine Issues and PRs related to the V8 dependency. label Feb 12, 2016

ofrobots force-pushed the vee-eight-4.9 branch from 2d3c7b0 to 720f9f0 Compare February 13, 2016 03:04

ofrobots added 3 commits February 18, 2016 12:12

src: replace usage of deprecated SetAccessor

23589ac

src: fix deprecated SetWeak usage in base-object

14ba959

buffer: cleanup CallbackInfo

34bf1da

Dynamic checks that CallbackInfo holds an ArrayBuffer handle can be converted into compiler enforced checks. Removed unused code, and other minor cleanup.

ofrobots force-pushed the vee-eight-4.9 branch from 7a580be to f5cc7af Compare February 18, 2016 20:27

bnoordhuis reviewed Feb 21, 2016
View reviewed changes

ofrobots force-pushed the vee-eight-4.9 branch from f5cc7af to 8be4985 Compare February 22, 2016 15:11

ofrobots mentioned this pull request Feb 23, 2016

(vee-eight-4.9) contextify: update deprecated SetWeak usage #5392

Closed

4 tasks

ofrobots force-pushed the vee-eight-4.9 branch from 8be4985 to dcecbde Compare February 24, 2016 17:31

ofrobots added a commit that referenced this pull request Feb 25, 2016

src: replace usage of deprecated SetAccessor

2e6934c

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit that referenced this pull request Feb 25, 2016

src: fix deprecated SetWeak usage in base-object

0aaacb5

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots closed this Feb 25, 2016

ofrobots added dont-land-on-v5.x labels Feb 25, 2016

ofrobots mentioned this pull request Feb 26, 2016

(vee-eight-4.9) src: update uses of deprecated NewExternal #5462

Closed

4 tasks

ofrobots added a commit to ofrobots/node that referenced this pull request Mar 1, 2016

src: replace usage of deprecated SetAccessor

81238bb

PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit to ofrobots/node that referenced this pull request Mar 1, 2016

src: fix deprecated SetWeak usage in base-object

502568e

PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit to ofrobots/node that referenced this pull request Mar 3, 2016

src: replace usage of deprecated SetAccessor

ea4f27c

PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit to ofrobots/node that referenced this pull request Mar 3, 2016

src: fix deprecated SetWeak usage in base-object

bdbd3a2

PR-URL: nodejs#5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit that referenced this pull request Mar 4, 2016

src: replace usage of deprecated SetAccessor

c1649a7

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots added a commit that referenced this pull request Mar 4, 2016

src: fix deprecated SetWeak usage in base-object

a45e1f9

PR-URL: #5204 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: indutny - Fedor Indutny <fedor.indutny@gmail.com>

ofrobots mentioned this pull request Mar 17, 2016

buffer: fix buffer alignment restriction #5752

Closed

3 tasks

lovell mentioned this pull request May 18, 2016

Understanding sharp mem usage with a disabled cache lovell/sharp#429

Closed

addaleax mentioned this pull request Jan 13, 2019

src: remove outdated Neuter() call in node_buffer.cc #25479

Closed

2 tasks

(vee-eight-4.9) Replace more deprecated V8 API usage by ofrobots · Pull Request #5204 · nodejs/node · GitHub

Uh oh!

Uh oh!

(vee-eight-4.9) Replace more deprecated V8 API usage - #5204

(vee-eight-4.9) Replace more deprecated V8 API usage#5204
ofrobots wants to merge 4 commits into
nodejs:vee-eight-4.9from
ofrobots:vee-eight-4.9

ofrobots commented Feb 12, 2016

Uh oh!

bnoordhuis Feb 21, 2016

Uh oh!

trevnorris Feb 21, 2016

Uh oh!

ofrobots Feb 22, 2016

Uh oh!

bnoordhuis commented Feb 21, 2016

Uh oh!

indutny commented Feb 21, 2016

Uh oh!

ofrobots commented Feb 22, 2016

Uh oh!

ofrobots commented Feb 25, 2016

Uh oh!

ofrobots commented Feb 25, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

(vee-eight-4.9) Replace more deprecated V8 API usage by ofrobots · Pull Request #5204 · nodejs/node · GitHub

Uh oh!

Uh oh!

(vee-eight-4.9) Replace more deprecated V8 API usage - #5204

(vee-eight-4.9) Replace more deprecated V8 API usage#5204ofrobots wants to merge 4 commits intonodejs:vee-eight-4.9nodejs/node:vee-eight-4.9from ofrobots:vee-eight-4.9ofrobots/node:vee-eight-4.9Copy head branch name to clipboard

Conversation

ofrobots commented Feb 12, 2016

Uh oh!

bnoordhuis Feb 21, 2016

Choose a reason for hiding this comment

Uh oh!

trevnorris Feb 21, 2016

Choose a reason for hiding this comment

Uh oh!

ofrobots Feb 22, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Feb 21, 2016

Uh oh!

indutny commented Feb 21, 2016

Uh oh!

ofrobots commented Feb 22, 2016

Uh oh!

ofrobots commented Feb 25, 2016

Uh oh!

ofrobots commented Feb 25, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

(vee-eight-4.9) Replace more deprecated V8 API usage#5204
ofrobots wants to merge 4 commits into
nodejs:vee-eight-4.9from
ofrobots:vee-eight-4.9