Skip to content

Commit 499fb42

Browse files
addaleaxtargos
authored andcommitted
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: #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]>
1 parent 5677235 commit 499fb42

File tree

1 file changed

+38
-5
lines changed

1 file changed

+38
-5
lines changed

src/js_native_api_v8.cc

+38-5
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ inline static napi_status ConcludeDeferred(napi_env env,
186186
}
187187

188188
// Wrapper around v8impl::Persistent that implements reference counting.
189-
class Reference : private Finalizer, RefTracker {
190-
private:
189+
class Reference : protected Finalizer, RefTracker {
190+
protected:
191191
Reference(napi_env env,
192192
v8::Local<v8::Value> value,
193193
uint32_t initial_refcount,
@@ -289,7 +289,7 @@ class Reference : private Finalizer, RefTracker {
289289
}
290290
}
291291

292-
private:
292+
protected:
293293
void Finalize(bool is_env_teardown = false) override {
294294
if (_finalize_callback != nullptr) {
295295
_env->CallIntoModuleThrow([&](napi_env env) {
@@ -310,6 +310,7 @@ class Reference : private Finalizer, RefTracker {
310310
}
311311
}
312312

313+
private:
313314
// The N-API finalizer callback may make calls into the engine. V8's heap is
314315
// not in a consistent state during the weak callback, and therefore it does
315316
// not support calls back into it. However, it provides a mechanism for adding
@@ -335,6 +336,37 @@ class Reference : private Finalizer, RefTracker {
335336
bool _delete_self;
336337
};
337338

339+
class ArrayBufferReference final : public Reference {
340+
public:
341+
// Same signatures for ctor and New() as Reference, except this only works
342+
// with ArrayBuffers:
343+
template <typename... Args>
344+
explicit ArrayBufferReference(napi_env env,
345+
v8::Local<v8::ArrayBuffer> value,
346+
Args&&... args)
347+
: Reference(env, value, std::forward<Args>(args)...) {}
348+
349+
template <typename... Args>
350+
static ArrayBufferReference* New(napi_env env,
351+
v8::Local<v8::ArrayBuffer> value,
352+
Args&&... args) {
353+
return new ArrayBufferReference(env, value, std::forward<Args>(args)...);
354+
}
355+
356+
private:
357+
void Finalize(bool is_env_teardown) override {
358+
if (is_env_teardown) {
359+
v8::HandleScope handle_scope(_env->isolate);
360+
v8::Local<v8::Value> ab = Get();
361+
CHECK(!ab.IsEmpty());
362+
CHECK(ab->IsArrayBuffer());
363+
ab.As<v8::ArrayBuffer>()->Detach();
364+
}
365+
366+
Reference::Finalize(is_env_teardown);
367+
}
368+
};
369+
338370
enum UnwrapAction {
339371
KeepWrap,
340372
RemoveWrap
@@ -2583,8 +2615,9 @@ napi_status napi_create_external_arraybuffer(napi_env env,
25832615

25842616
if (finalize_cb != nullptr) {
25852617
// Create a self-deleting weak reference that invokes the finalizer
2586-
// callback.
2587-
v8impl::Reference::New(env,
2618+
// callback and detaches the ArrayBuffer if it still exists on Environment
2619+
// teardown.
2620+
v8impl::ArrayBufferReference::New(env,
25882621
buffer,
25892622
0,
25902623
true,

0 commit comments

Comments
 (0)