From 342dbff852fb2ec54c2aec8c5886e2401c081e8d 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.

PR-URL: https://github.com/nodejs/node/pull/20633
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 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 a8f0fdca40400a..3322f9ebcd366d 100644
--- a/src/node.h
+++ b/src/node.h
@@ -733,7 +733,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();