It's recommended to add v8::HandleScope for every exposed method?

158 views
Skip to first unread message

Jonathan Cardoso

unread,
Apr 18, 2014, 10:13:56 PM4/18/14
to nod...@googlegroups.com
I've forked a library for a native module that had not received any updates for a while, as I'm learning some C++, I will try to learn working on this module.

And taking a look at the code, I just hit a question, there is no v8::HandleScope on the exposed methods, this can cause something unexpected?

Code at https://github.com/JCMais/node-libcurl/blob/develop/src/node-curl.cc


mscdex

unread,
Apr 18, 2014, 10:29:32 PM4/18/14
to nod...@googlegroups.com
On Friday, April 18, 2014 10:13:56 PM UTC-4, Jonathan Cardoso wrote:
I've forked a library for a native module that had not received any updates for a while, as I'm learning some C++, I will try to learn working on this module.

And taking a look at the code, I just hit a question, there is no v8::HandleScope on the exposed methods, this can cause something unexpected?


It's best to have a HandleScope (at the top) in any function where v8 handles/values/etc. are being created and/or used. Also lines like [1] should use scope.Close() around the value instead of returning it directly. It should be noted that the v8 API has changed a fair amount post-node-v0.10.x.

[1] https://github.com/JCMais/node-libcurl/blob/develop/src/node-curl.cc#L322

Jonathan Cardoso

unread,
Apr 18, 2014, 11:36:44 PM4/18/14
to nod...@googlegroups.com
Nice to know, thank you. I will add those after I've the test suite working.

I read about the V8 changes, I've already plans to port the code to use NAN.

Rod Vagg

unread,
Apr 20, 2014, 5:33:10 AM4/20/14
to nod...@googlegroups.com

Please note if you are using NAN we have a new major release coming up. Probably 1.0.0 but currently slated as 0.9.0, you can see the details here: https://github.com/rvagg/nan/pull/86

There are some major changes in the last couple of versions of V8 that have made us shift the API, plus we've also taken the chance to tighten a bunch of things up so overall it's a much nicer API to develop against. As usual the aim is to provide a single API to develop addons against that give you compatibility all the way back to 0.8 and up to 0.12, but we have to shift that API occasionally to make accommodations for new upstream changes. We'll likely be targeting a release at or during the 0.11.13 release and when we do we'll be removing compatibility with all previous 0.11.x releases (note that 0.11.11 and 0.11.12 are broken for all native addons anyway).

An example of an upgrade and the changes that you'd need to make if you're already using NAN can be seen in this commit for a currently unreleased version of LevelDOWN (current WIP branch is using git://github.com/rvagg/nan.git#0.9-wip as the dependency, you can develop against that but please don't release with that!).

If you are writing native addons and not preparing for 0.12 then you really need to start investigating what you're going to do about it, there are some huge changes to the V8 API that are going to cause a lot of pain. There will be much wailing and gnashing of teeth come 0.12 if everyone leaves it too late.

If you are using a native addon that doesn't currently have 0.11 support (disregarding 0.11.11 and 0.11.12) then you should be applying some pressure so we can minimise some of the upgrade pain we're likely to experience.

Regarding HandleScope specifically, there is a new EscapableHandleScope in V8 for returning values that replaces the old HandleScope#Close(value) usage, we're exposing it here.

Cheers,
 -- Rod
Reply all
Reply to author
Forward
0 new messages