Skip to content

Commit

Permalink
n-api: detach external ArrayBuffers on env exit
Browse files Browse the repository at this point in the history
Make sure that `ArrayBuffer`s created using
`napi_create_external_arraybuffer` are rendered unusable
after its memory has been released.

PR-URL: #30551
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
addaleax authored and MylesBorins committed Dec 17, 2019
1 parent 42cb92f commit dc9c770
Showing 1 changed file with 38 additions and 5 deletions.
43 changes: 38 additions & 5 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit dc9c770

Please sign in to comment.