async_wrap: ensure all objects have internal field

If the constructor can't assign a class id then the heap snapshot will not be able to report the object. So ensure that all AsyncWrap instances use a FunctionTemplate instance with an internal field count >= 1. PR-URL: #3139 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-by: Stephen Belanger <admin@stephenbelanger.com>
nodejs · Oct 2, 2015 · 39b8730 · 39b8730
1 parent 99e6607
commit 39b8730
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 12 deletions.
diff --git a/lib/dns.js b/lib/dns.js
@@ -8,6 +8,7 @@ const uv = process.binding('uv');
 
 const GetAddrInfoReqWrap = cares.GetAddrInfoReqWrap;
 const GetNameInfoReqWrap = cares.GetNameInfoReqWrap;
+const QueryReqWrap = cares.QueryReqWrap;
 
 const isIp = net.isIP;
 
@@ -223,12 +224,11 @@ function resolver(bindingName) {
     }
 
     callback = makeAsync(callback);
-    var req = {
-      bindingName: bindingName,
-      callback: callback,
-      hostname: name,
-      oncomplete: onresolve
-    };
+    var req = new QueryReqWrap();
+    req.bindingName = bindingName;
+    req.callback = callback;
+    req.hostname = name;
+    req.oncomplete = onresolve;
     var err = binding(req, name);
     if (err) throw errnoException(err, bindingName);
     callback.immediately = true;

diff --git a/src/async-wrap-inl.h b/src/async-wrap-inl.h
@@ -18,10 +18,11 @@ inline AsyncWrap::AsyncWrap(Environment* env,
                             ProviderType provider,
                             AsyncWrap* parent)
     : BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1) {
-  // Only set wrapper class id if object will be Wrap'd.
-  if (object->InternalFieldCount() > 0)
-    // Shift provider value over to prevent id collision.
-    persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
+  CHECK_NE(provider, PROVIDER_NONE);
+  CHECK_GE(object->InternalFieldCount(), 1);
+
+  // Shift provider value over to prevent id collision.
+  persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);
 
   // Check user controlled flag to see if the init callback should run.
   if (!env->using_asyncwrap())

diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc
@@ -85,6 +85,11 @@ static void NewGetNameInfoReqWrap(const FunctionCallbackInfo<Value>& args) {
 }
 
 
+static void NewQueryReqWrap(const FunctionCallbackInfo<Value>& args) {
+  CHECK(args.IsConstructCall());
+}
+
+
 static int cmp_ares_tasks(const ares_task_t* a, const ares_task_t* b) {
   if (a->sock < b->sock)
     return -1;
@@ -1312,6 +1317,14 @@ static void Initialize(Local<Object> target,
       FIXED_ONE_BYTE_STRING(env->isolate(), "GetNameInfoReqWrap"));
   target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "GetNameInfoReqWrap"),
               niw->GetFunction());
+
+  Local<FunctionTemplate> qrw =
+      FunctionTemplate::New(env->isolate(), NewQueryReqWrap);
+  qrw->InstanceTemplate()->SetInternalFieldCount(1);
+  qrw->SetClassName(
+      FIXED_ONE_BYTE_STRING(env->isolate(), "QueryReqWrap"));
+  target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "QueryReqWrap"),
+              qrw->GetFunction());
 }
 
 }  // namespace cares_wrap

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -221,6 +221,13 @@ inline Environment::Environment(v8::Local<v8::Context> context,
   set_as_external(v8::External::New(isolate(), this));
   set_binding_cache_object(v8::Object::New(isolate()));
   set_module_load_list_array(v8::Array::New(isolate()));
+
+  v8::Local<v8::FunctionTemplate> fn = v8::FunctionTemplate::New(isolate());
+  fn->SetClassName(FIXED_ONE_BYTE_STRING(isolate(), "InternalFieldObject"));
+  v8::Local<v8::ObjectTemplate> obj = fn->InstanceTemplate();
+  obj->SetInternalFieldCount(1);
+  set_generic_internal_field_template(obj);
+
   RB_INIT(&cares_task_list_);
   handle_cleanup_waiting_ = 0;
 }
@@ -513,6 +520,12 @@ inline void Environment::SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
   function->SetName(name_string);  // NODE_SET_METHOD() compatibility.
 }
 
+inline v8::Local<v8::Object> Environment::NewInternalFieldObject() {
+  v8::MaybeLocal<v8::Object> m_obj =
+      generic_internal_field_template()->NewInstance(context());
+  return m_obj.ToLocalChecked();
+}
+
 #define V(PropertyName, StringValue)                                          \
   inline                                                                      \
   v8::Local<v8::String> Environment::IsolateData::PropertyName() const {      \

diff --git a/src/env.h b/src/env.h
@@ -237,6 +237,7 @@ namespace node {
   V(context, v8::Context)                                                     \
   V(domain_array, v8::Array)                                                  \
   V(fs_stats_constructor_function, v8::Function)                              \
+  V(generic_internal_field_template, v8::ObjectTemplate)                      \
   V(jsstream_constructor_template, v8::FunctionTemplate)                      \
   V(module_load_list_array, v8::Array)                                        \
   V(pipe_constructor_template, v8::FunctionTemplate)                          \
@@ -488,6 +489,7 @@ class Environment {
                                 const char* name,
                                 v8::FunctionCallback callback);
 
+  inline v8::Local<v8::Object> NewInternalFieldObject();
 
   // Strings are shared across shared contexts. The getters simply proxy to
   // the per-isolate primitive.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -4624,6 +4624,7 @@ class PBKDF2Request : public AsyncWrap {
         iter_(iter) {
     if (key() == nullptr)
       FatalError("node::PBKDF2Request()", "Out of Memory");
+    Wrap(object, this);
   }
 
   ~PBKDF2Request() override {
@@ -4833,7 +4834,7 @@ void PBKDF2(const FunctionCallbackInfo<Value>& args) {
     digest = EVP_sha1();
   }
 
-  obj = Object::New(env->isolate());
+  obj = env->NewInternalFieldObject();
   req = new PBKDF2Request(env,
                           obj,
                           digest,
@@ -4885,6 +4886,7 @@ class RandomBytesRequest : public AsyncWrap {
         data_(static_cast<char*>(malloc(size))) {
     if (data() == nullptr)
       FatalError("node::RandomBytesRequest()", "Out of Memory");
+    Wrap(object, this);
   }
 
   ~RandomBytesRequest() override {
@@ -5001,7 +5003,7 @@ void RandomBytes(const FunctionCallbackInfo<Value>& args) {
   if (size < 0 || size > Buffer::kMaxLength)
     return env->ThrowRangeError("size is not a valid Smi");
 
-  Local<Object> obj = Object::New(env->isolate());
+  Local<Object> obj = env->NewInternalFieldObject();
   RandomBytesRequest* req = new RandomBytesRequest(env, obj, size);
 
   if (args[1]->IsFunction()) {