Skip to content

Commit

Permalink
buffer,n-api: fix double ArrayBuffer::Detach() during cleanup
Browse files Browse the repository at this point in the history
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 4f523c2 and could
be ported back to v12.x with a backport of this PR.

Fixes: #33022
Refs: #30551

PR-URL: #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>
  • Loading branch information
addaleax authored and BridgeAR committed Apr 28, 2020
1 parent dd1cc1a commit 18fd841
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
10 changes: 6 additions & 4 deletions src/js_native_api_v8.cc
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/node_buffer.cc
Expand Up @@ -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());
Expand Down

0 comments on commit 18fd841

Please sign in to comment.