From 18fd841ab494f122579ec310eab7d9b3bc5364a7 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 24 Apr 2020 15:45:51 +0200
Subject: [PATCH] buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These calls could fail if the `ArrayBuffer` had already been explicitly
detached at some point in the past.

The necessary test changes already came with 4f523c2c1a1c and could
be ported back to v12.x with a backport of this PR.

Fixes: https://github.com/nodejs/node/issues/33022
Refs: https://github.com/nodejs/node/pull/30551

PR-URL: https://github.com/nodejs/node/pull/33039
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
---
 src/js_native_api_v8.cc | 10 ++++++----
 src/node_buffer.cc      |  3 ++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
index 1a01f2a29081ff..5e8794e8d0cd1c 100644
--- a/src/js_native_api_v8.cc
+++ b/src/js_native_api_v8.cc
@@ -392,10 +392,12 @@ class ArrayBufferReference final : public Reference {
   inline 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();
+      v8::Local<v8::Value> obj = Get();
+      CHECK(!obj.IsEmpty());
+      CHECK(obj->IsArrayBuffer());
+      v8::Local<v8::ArrayBuffer> ab = obj.As<v8::ArrayBuffer>();
+      if (ab->IsDetachable())
+        ab->Detach();
     }
 
     Reference::Finalize(is_env_teardown);
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 9da95a350b0c52..374e4c52cc234e 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -152,7 +152,8 @@ void CallbackInfo::CleanupHook(void* data) {
     HandleScope handle_scope(self->env_->isolate());
     Local<ArrayBuffer> ab = self->persistent_.Get(self->env_->isolate());
     CHECK(!ab.IsEmpty());
-    ab->Detach();
+    if (ab->IsDetachable())
+      ab->Detach();
   }
 
   self->WeakCallback(self->env_->isolate());