From 00f395db45f20c4fd2b8ea48a6b97b5b72494c4c 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 23e2e9995ce..5d6aa8dadfc 100644
--- a/src/node.h
+++ b/src/node.h
@@ -712,7 +712,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 857d74c2e62..ab33858c233 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 f19ad58637f..c37d4df83d0 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();