From fa5937b1e75ab04c1a1aaf396b36fbfaf4fff155 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Nov 2019 21:36:41 +0100 Subject: 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 Reviewed-By: Colin Ihrig Reviewed-By: Denys Otrishko Reviewed-By: Michael Dawson --- 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 05712afde0..5506b2b4c6 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 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 + explicit ArrayBufferReference(napi_env env, + v8::Local value, + Args&&... args) + : Reference(env, value, std::forward(args)...) {} + + template + static ArrayBufferReference* New(napi_env env, + v8::Local value, + Args&&... args) { + return new ArrayBufferReference(env, value, std::forward(args)...); + } + + private: + void Finalize(bool is_env_teardown) override { + if (is_env_teardown) { + v8::HandleScope handle_scope(_env->isolate); + v8::Local ab = Get(); + CHECK(!ab.IsEmpty()); + CHECK(ab->IsArrayBuffer()); + ab.As()->Detach(); + } + + Reference::Finalize(is_env_teardown); + } +}; + enum UnwrapAction { KeepWrap, RemoveWrap @@ -2587,8 +2619,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, -- cgit v1.2.3