From 4386d67082a558fba2d12db072aa8d7366a68f7a Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Wed, 9 May 2018 18:32:59 +0200
Subject: [PATCH] src: make `AsyncResource` destructor virtual
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`AsyncResource` is intended to be a base class, and since we don’t
know what API consumers will do with it in their own code,
it’s good practice to make its destructor virtual.

This should not be ABI-breaking since all class methods are inline.
---
 src/node.h                            |  2 +-
 test/addons/async-resource/binding.cc | 22 ++++++++++++++++++++++
 test/addons/async-resource/test.js    |  2 ++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/node.h b/src/node.h
index 5a491c1abf5457..0fea0d849587a0 100644
--- a/src/node.h
+++ b/src/node.h
@@ -699,7 +699,7 @@ class AsyncResource {
                                      trigger_async_id);
     }
 
-    ~AsyncResource() {
+    virtual ~AsyncResource() {
       EmitAsyncDestroy(isolate_, async_context_);
     }
 
diff --git a/test/addons/async-resource/binding.cc b/test/addons/async-resource/binding.cc
index 857d74c2e62206..ab33858c233dd0 100644
--- a/test/addons/async-resource/binding.cc
+++ b/test/addons/async-resource/binding.cc
@@ -17,6 +17,17 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
+int custom_async_resource_destructor_calls = 0;
+
+class CustomAsyncResource : public AsyncResource {
+ public:
+  CustomAsyncResource(Isolate* isolate, Local<Object> resource)
+      : AsyncResource(isolate, resource, "CustomAsyncResource") {}
+  ~CustomAsyncResource() {
+    custom_async_resource_destructor_calls++;
+  }
+};
+
 void CreateAsyncResource(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   assert(args[0]->IsObject());
@@ -98,6 +109,16 @@ void GetResource(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(r->get_resource());
 }
 
+void RunSubclassTest(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Object> obj = Object::New(isolate);
+
+  assert(custom_async_resource_destructor_calls == 0);
+  CustomAsyncResource* resource = new CustomAsyncResource(isolate, obj);
+  delete static_cast<AsyncResource*>(resource);
+  assert(custom_async_resource_destructor_calls == 1);
+}
+
 void Initialize(Local<Object> exports) {
   NODE_SET_METHOD(exports, "createAsyncResource", CreateAsyncResource);
   NODE_SET_METHOD(exports, "destroyAsyncResource", DestroyAsyncResource);
@@ -107,6 +128,7 @@ void Initialize(Local<Object> exports) {
   NODE_SET_METHOD(exports, "getAsyncId", GetAsyncId);
   NODE_SET_METHOD(exports, "getTriggerAsyncId", GetTriggerAsyncId);
   NODE_SET_METHOD(exports, "getResource", GetResource);
+  NODE_SET_METHOD(exports, "runSubclassTest", RunSubclassTest);
 }
 
 }  // anonymous namespace
diff --git a/test/addons/async-resource/test.js b/test/addons/async-resource/test.js
index f19ad58637f187..c37d4df83d0103 100644
--- a/test/addons/async-resource/test.js
+++ b/test/addons/async-resource/test.js
@@ -5,6 +5,8 @@ const assert = require('assert');
 const binding = require(`./build/${common.buildType}/binding`);
 const async_hooks = require('async_hooks');
 
+binding.runSubclassTest();
+
 const kObjectTag = Symbol('kObjectTag');
 const rootAsyncId = async_hooks.executionAsyncId();