From 499fb42afe1ab2df015129ff321ed978d50681c4 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 19 Nov 2019 21:36:41 +0100
Subject: [PATCH] n-api: detach external ArrayBuffers on env exit

Make sure that `ArrayBuffer`s created using
`napi_create_external_arraybuffer` are rendered unusable
after its memory has been released.

PR-URL: https://github.com/nodejs/node/pull/30551
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 src/js_native_api_v8.cc | 43 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
index 2febae1f1347a0..ed220e4748922c 100644
--- a/src/js_native_api_v8.cc
+++ b/src/js_native_api_v8.cc
@@ -186,8 +186,8 @@ inline static napi_status ConcludeDeferred(napi_env env,
 }
 
 // Wrapper around v8impl::Persistent that implements reference counting.
-class Reference : private Finalizer, RefTracker {
- private:
+class Reference : protected Finalizer, RefTracker {
+ protected:
   Reference(napi_env env,
             v8::Local<v8::Value> value,
             uint32_t initial_refcount,
@@ -289,7 +289,7 @@ class Reference : private Finalizer, RefTracker {
     }
   }
 
- private:
+ protected:
   void Finalize(bool is_env_teardown = false) override {
     if (_finalize_callback != nullptr) {
       _env->CallIntoModuleThrow([&](napi_env env) {
@@ -310,6 +310,7 @@ class Reference : private Finalizer, RefTracker {
     }
   }
 
+ private:
   // The N-API finalizer callback may make calls into the engine. V8's heap is
   // not in a consistent state during the weak callback, and therefore it does
   // not support calls back into it. However, it provides a mechanism for adding
@@ -335,6 +336,37 @@ class Reference : private Finalizer, RefTracker {
   bool _delete_self;
 };
 
+class ArrayBufferReference final : public Reference {
+ public:
+  // Same signatures for ctor and New() as Reference, except this only works
+  // with ArrayBuffers:
+  template <typename... Args>
+  explicit ArrayBufferReference(napi_env env,
+                                v8::Local<v8::ArrayBuffer> value,
+                                Args&&... args)
+    : Reference(env, value, std::forward<Args>(args)...) {}
+
+  template <typename... Args>
+  static ArrayBufferReference* New(napi_env env,
+                                   v8::Local<v8::ArrayBuffer> value,
+                                   Args&&... args) {
+    return new ArrayBufferReference(env, value, std::forward<Args>(args)...);
+  }
+
+ private:
+  void Finalize(bool is_env_teardown) override {
+    if (is_env_teardown) {
+      v8::HandleScope handle_scope(_env->isolate);
+      v8::Local<v8::Value> ab = Get();
+      CHECK(!ab.IsEmpty());
+      CHECK(ab->IsArrayBuffer());
+      ab.As<v8::ArrayBuffer>()->Detach();
+    }
+
+    Reference::Finalize(is_env_teardown);
+  }
+};
+
 enum UnwrapAction {
   KeepWrap,
   RemoveWrap
@@ -2583,8 +2615,9 @@ napi_status napi_create_external_arraybuffer(napi_env env,
 
   if (finalize_cb != nullptr) {
     // Create a self-deleting weak reference that invokes the finalizer
-    // callback.
-    v8impl::Reference::New(env,
+    // callback and detaches the ArrayBuffer if it still exists on Environment
+    // teardown.
+    v8impl::ArrayBufferReference::New(env,
         buffer,
         0,
         true,