From a7988ab0fa3b018068f2706ae4cfc3a5ab07948c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 18 Dec 2019 16:47:38 +0100 Subject: [PATCH] doc: avoid using v8::Persistent in addon docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use `v8::Global` where possible. For examples where it applies, also clean up the code and make the code multi-threading-ready, for those where that isn’t easily possible, add a warning about that. PR-URL: https://github.com/nodejs/node/pull/31018 Reviewed-By: Colin Ihrig Reviewed-By: Richard Lau Reviewed-By: James M Snell Reviewed-By: David Carlier Reviewed-By: Rich Trott Reviewed-By: Gus Caplan Reviewed-By: Joyee Cheung Reviewed-By: Ben Noordhuis Reviewed-By: Stephen Belanger --- doc/api/addons.md | 63 ++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/doc/api/addons.md b/doc/api/addons.md index 8b17d6b417b8e8..3b33b9cffe5094 100644 --- a/doc/api/addons.md +++ b/doc/api/addons.md @@ -157,11 +157,11 @@ The context-aware addon can be structured to avoid global static data by performing the following steps: * defining a class which will hold per-addon-instance data. Such -a class should include a `v8::Persistent` which will hold a weak +a class should include a `v8::Global` which will hold a weak reference to the addon's `exports` object. The callback associated with the weak reference will then destroy the instance of the class. * constructing an instance of this class in the addon initializer such that the -`v8::Persistent` is set to the `exports` object. +`v8::Global` is set to the `exports` object. * storing the instance of the class in a `v8::External`, and * passing the `v8::External` to all methods exposed to JavaScript by passing it to the `v8::FunctionTemplate` constructor which creates the native-backed @@ -188,14 +188,6 @@ class AddonData { exports_.SetWeak(this, DeleteMe, WeakCallbackType::kParameter); } - ~AddonData() { - if (!exports_.IsEmpty()) { - // Reset the reference to avoid leaking data. - exports_.ClearWeak(); - exports_.Reset(); - } - } - // Per-addon data. int call_count; @@ -207,7 +199,7 @@ class AddonData { // Weak handle to the "exports" object. An instance of this class will be // destroyed along with the exports object to which it is weakly bound. - v8::Persistent exports_; + v8::Global exports_; }; static void Method(const v8::FunctionCallbackInfo& info) { @@ -830,7 +822,7 @@ class MyObject : public node::ObjectWrap { static void New(const v8::FunctionCallbackInfo& args); static void PlusOne(const v8::FunctionCallbackInfo& args); - static v8::Persistent constructor; + double value_; }; @@ -858,12 +850,10 @@ using v8::Local; using v8::NewStringType; using v8::Number; using v8::Object; -using v8::Persistent; +using v8::ObjectTemplate; using v8::String; using v8::Value; -Persistent MyObject::constructor; - MyObject::MyObject(double value) : value_(value) { } @@ -872,9 +862,15 @@ MyObject::~MyObject() { void MyObject::Init(Local exports) { Isolate* isolate = exports->GetIsolate(); + Local context = isolate->GetCurrentContext(); + + Local addon_data_tpl = ObjectTemplate::New(isolate); + addon_data_tpl->SetInternalFieldCount(1); // 1 field for the MyObject::New() + Local addon_data = + addon_data_tpl->NewInstance(context).ToLocalChecked(); // Prepare constructor template - Local tpl = FunctionTemplate::New(isolate, New); + Local tpl = FunctionTemplate::New(isolate, New, addon_data); tpl->SetClassName(String::NewFromUtf8( isolate, "MyObject", NewStringType::kNormal).ToLocalChecked()); tpl->InstanceTemplate()->SetInternalFieldCount(1); @@ -882,11 +878,11 @@ void MyObject::Init(Local exports) { // Prototype NODE_SET_PROTOTYPE_METHOD(tpl, "plusOne", PlusOne); - Local context = isolate->GetCurrentContext(); - constructor.Reset(isolate, tpl->GetFunction(context).ToLocalChecked()); + Local constructor = tpl->GetFunction(context).ToLocalChecked(); + addon_data->SetInternalField(0, constructor); exports->Set(context, String::NewFromUtf8( isolate, "MyObject", NewStringType::kNormal).ToLocalChecked(), - tpl->GetFunction(context).ToLocalChecked()).FromJust(); + constructor).FromJust(); } void MyObject::New(const FunctionCallbackInfo& args) { @@ -904,7 +900,8 @@ void MyObject::New(const FunctionCallbackInfo& args) { // Invoked as plain function `MyObject(...)`, turn into construct call. const int argc = 1; Local argv[argc] = { args[0] }; - Local cons = Local::New(isolate, constructor); + Local cons = + args.Data().As()->GetInternalField(0).As(); Local result = cons->NewInstance(context, argc, argv).ToLocalChecked(); args.GetReturnValue().Set(result); @@ -1029,7 +1026,7 @@ class MyObject : public node::ObjectWrap { static void New(const v8::FunctionCallbackInfo& args); static void PlusOne(const v8::FunctionCallbackInfo& args); - static v8::Persistent constructor; + static v8::Global constructor; double value_; }; @@ -1047,20 +1044,23 @@ The implementation in `myobject.cc` is similar to the previous example: namespace demo { +using node::AddEnvironmentCleanupHook; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::Isolate; using v8::Local; using v8::NewStringType; using v8::Number; using v8::Object; -using v8::Persistent; using v8::String; using v8::Value; -Persistent MyObject::constructor; +// Warning! This is not thread-safe, this addon cannot be used for worker +// threads. +Global MyObject::constructor; MyObject::MyObject(double value) : value_(value) { } @@ -1080,6 +1080,10 @@ void MyObject::Init(Isolate* isolate) { Local context = isolate->GetCurrentContext(); constructor.Reset(isolate, tpl->GetFunction(context).ToLocalChecked()); + + AddEnvironmentCleanupHook(isolate, [](void*) { + constructor.Reset(); + }, nullptr); } void MyObject::New(const FunctionCallbackInfo& args) { @@ -1246,7 +1250,7 @@ class MyObject : public node::ObjectWrap { ~MyObject(); static void New(const v8::FunctionCallbackInfo& args); - static v8::Persistent constructor; + static v8::Global constructor; double value_; }; @@ -1264,19 +1268,22 @@ The implementation of `myobject.cc` is similar to before: namespace demo { +using node::AddEnvironmentCleanupHook; using v8::Context; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; +using v8::Global; using v8::Isolate; using v8::Local; using v8::NewStringType; using v8::Object; -using v8::Persistent; using v8::String; using v8::Value; -Persistent MyObject::constructor; +// Warning! This is not thread-safe, this addon cannot be used for worker +// threads. +Global MyObject::constructor; MyObject::MyObject(double value) : value_(value) { } @@ -1293,6 +1300,10 @@ void MyObject::Init(Isolate* isolate) { Local context = isolate->GetCurrentContext(); constructor.Reset(isolate, tpl->GetFunction(context).ToLocalChecked()); + + AddEnvironmentCleanupHook(isolate, [](void*) { + constructor.Reset(); + }, nullptr); } void MyObject::New(const FunctionCallbackInfo& args) {