Skip to content

Commit be9788b

Browse files
committed
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 e8af569 commit be9788b

File tree

1 file changed

+38
-5
lines changed

1 file changed

+38
-5
lines changed

‎src/js_native_api_v8.cc‎

Lines changed: 38 additions & 5 deletions
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-
classReference : privateFinalizer, RefTracker{
190-
private:
189+
classReference : protectedFinalizer, 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
voidFinalize(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+
classArrayBufferReferencefinal : 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+
explicitArrayBufferReference(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+
returnnewArrayBufferReference(env, value, std::forward<Args>(args)...);
354+
}
355+
356+
private:
357+
voidFinalize(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
@@ -2587,8 +2619,9 @@ napi_status napi_create_external_arraybuffer(napi_env env,
25872619

25882620
if (finalize_cb != nullptr){
25892621
// Create a self-deleting weak reference that invokes the finalizer
2590-
// callback.
2591-
v8impl::Reference::New(env,
2622+
// callback and detaches the ArrayBuffer if it still exists on Environment
2623+
// teardown.
2624+
v8impl::ArrayBufferReference::New(env,
25922625
buffer,
25932626
0,
25942627
true,

0 commit comments

Comments
(0)