Skip to content

Allow SetWeak() for non-object persistent handles.#824

Merged
kkoopa merged 1 commit into
nodejs:masterfrom
bnoordhuis:non-object-setweak
Nov 20, 2018
Merged

Allow SetWeak() for non-object persistent handles.#824
kkoopa merged 1 commit into
nodejs:masterfrom
bnoordhuis:non-object-setweak

Conversation

@bnoordhuis

Copy link
Copy Markdown
Member

The definition of the SetWeak() template method constrained the type to
types with a GetAlignedPointerFromInternalField() method. In practice
that meant it only worked with T=v8::Object.

Lift that restriction so that e.g. T=v8::External now also works. Note
that SetWeak(WeakCallbackType::kInternalFields) for types without the
aforementioned method will result in a run-time assertion.


I wrote this over a year ago but I forgot to PR it. I don't need it now but I did want it at the time and it still seems useful so here goes.

@bnoordhuis

Copy link
Copy Markdown
Member Author

CI failure is a node-gyp incompatibility with Node.js v0.8.28 (!):

Details
gyp info it worked if it ends with ok
gyp info using node-gyp@3.6.3
gyp info using node@0.8.28 | darwin | x64
gyp info chdir test
gyp ERR! UNCAUGHT EXCEPTION 
gyp ERR! stack TypeError: Cannot read property 'prototype' of undefined
gyp ERR! stack     at Object.exports.inherits (util.js:538:43)
gyp ERR! stack     at Object.<anonymous> (/Users/travis/build/nodejs/nan/node_modules/request/node_modules/http-signature/node_modules/sshpk/lib/ed-compat.js:25:6)
gyp ERR! stack     at Module._compile (module.js:449:26)
gyp ERR! stack     at Object.Module._extensions..js (module.js:467:10)
gyp ERR! stack     at Module.load (module.js:356:32)
gyp ERR! stack     at Function.Module._load (module.js:312:12)
gyp ERR! stack     at Module.require (module.js:362:17)
gyp ERR! stack     at require (module.js:378:17)
gyp ERR! stack     at Object.<anonymous> (/Users/travis/build/nodejs/nan/node_modules/request/node_modules/http-signature/node_modules/sshpk/lib/private-key.js:17:16)
gyp ERR! stack     at Module._compile (module.js:449:26)
gyp ERR! System Darwin 17.4.0
gyp ERR! command "node" "/Users/travis/build/nodejs/nan/node_modules/.bin/node-gyp" "rebuild" "--directory" "test"
gyp ERR! cwd /Users/travis/build/nodejs/nan/test
gyp ERR! node -v v0.8.28
gyp ERR! node-gyp -v v3.6.3

The definition of the SetWeak() template method constrained the type to
types with a GetAlignedPointerFromInternalField() method.  In practice
that meant it only worked with T=v8::Object.

Lift that restriction so that e.g. T=v8::External now also works.  Note
that `SetWeak(WeakCallbackType::kInternalFields)` for types without the
aforementioned method will result in a run-time assertion.
@bnoordhuis

Copy link
Copy Markdown
Member Author

Rebased onto master. CI should be passing now that #826 is merged.

@kkoopa

kkoopa commented Nov 20, 2018

Copy link
Copy Markdown
Collaborator

LGTM

@kkoopa kkoopa merged commit e6ef6a4 into nodejs:master Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants