src: make AsyncResource destructor virtual

`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: #20633
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: addaleax

src/node.h

@@ -712,7 +712,7 @@ class AsyncResource {
                                      trigger_async_id);
     }
 
-    ~AsyncResource() {
+    virtual ~AsyncResource() {
       EmitAsyncDestroy(isolate_, async_context_);
     }
 

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

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();