mirror of
https://github.com/nodejs/node.git
synced 2025-08-15 13:48:44 +02:00
deps: V8: backport 954187bb1b87
Original commit message: [api] add Isolate::Deinitialize() and Isolate::Free() This allows embedders to mirror the isolate disposal routine with an initialization routine that uses Isolate::Allocate(). ``` v8::Isolate* isolate = v8::Isolate::Allocate(); // Use the isolate address as a key. v8::Isolate::Initialize(isolate, params); isolate->Deinitialize(); // Remove the entry keyed by isolate address. v8::Isolate::Free(isolate); ``` Previously, the only way to dispose the isolate bundles the de-initialization and the freeing of the address together in v8::Isolate::Dispose(). This is inadequate for embedders like Node.js that uses the isolate address as a key to manage the task runner associated with it, if another thread gets an isolate allocated at the aligned address before the other thread finishes cleanup for the isolate previously allocated at the same address, and locking on the entire disposal can be too risky since it may post GC tasks that in turn requires using the isolate address to locate the task runner. It's a lot simpler to handle the issue if the disposal process of the isolate can mirror the initialization of it and split into two routines. Refs: https://github.com/nodejs/node/pull/57753#issuecomment-2818999420 Refs: https://github.com/nodejs/node/issues/30850 Bug: 412943769 Change-Id: I3865c27395aded3a6f32de74d96d0698b2d891b9 Reviewed-on:6480071
Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Joyee Cheung <joyee@igalia.com> Cr-Commit-Position: refs/heads/main@{#99890} Refs:954187bb1b
PR-URL: https://github.com/nodejs/node/pull/58070 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
This commit is contained in:
parent
0f98039268
commit
d58f474b2d
6 changed files with 74 additions and 8 deletions
|
@ -38,7 +38,7 @@
|
|||
|
||||
# Reset this number to 0 on major V8 upgrades.
|
||||
# Increment by one for each non-official patch applied to deps/v8.
|
||||
'v8_embedder_string': '-node.9',
|
||||
'v8_embedder_string': '-node.10',
|
||||
|
||||
##### V8 defaults for Node.js #####
|
||||
|
||||
|
|
17
deps/v8/include/v8-isolate.h
vendored
17
deps/v8/include/v8-isolate.h
vendored
|
@ -872,11 +872,26 @@ class V8_EXPORT Isolate {
|
|||
void Exit();
|
||||
|
||||
/**
|
||||
* Disposes the isolate. The isolate must not be entered by any
|
||||
* Deinitializes and frees the isolate. The isolate must not be entered by any
|
||||
* thread to be disposable.
|
||||
*/
|
||||
void Dispose();
|
||||
|
||||
/**
|
||||
* Deinitializes the isolate, but does not free the address. The isolate must
|
||||
* not be entered by any thread to be deinitializable. Embedders must call
|
||||
* Isolate::Free() to free the isolate afterwards.
|
||||
*/
|
||||
void Deinitialize();
|
||||
|
||||
/**
|
||||
* Frees the memory allocated for the isolate. Can only be called after the
|
||||
* Isolate has already been deinitialized with Isolate::Deinitialize(). After
|
||||
* the isolate is freed, the next call to Isolate::New() or
|
||||
* Isolate::Allocate() might return the same address that just get freed.
|
||||
*/
|
||||
static void Free(Isolate* isolate);
|
||||
|
||||
/**
|
||||
* Dumps activated low-level V8 internal stats. This can be used instead
|
||||
* of performing a full isolate disposal.
|
||||
|
|
15
deps/v8/src/api/api.cc
vendored
15
deps/v8/src/api/api.cc
vendored
|
@ -10132,6 +10132,21 @@ void Isolate::Dispose() {
|
|||
i::Isolate::Delete(i_isolate);
|
||||
}
|
||||
|
||||
void Isolate::Deinitialize() {
|
||||
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
|
||||
if (!Utils::ApiCheck(
|
||||
!i_isolate->IsInUse(), "v8::Isolate::Deinitialize()",
|
||||
"Deinitializing the isolate that is entered by a thread")) {
|
||||
return;
|
||||
}
|
||||
i::Isolate::Deinitialize(i_isolate);
|
||||
}
|
||||
|
||||
void Isolate::Free(Isolate* isolate) {
|
||||
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
|
||||
i::Isolate::Free(i_isolate);
|
||||
}
|
||||
|
||||
void Isolate::DumpAndResetStats() {
|
||||
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(this);
|
||||
i_isolate->DumpAndResetStats();
|
||||
|
|
15
deps/v8/src/execution/isolate.cc
vendored
15
deps/v8/src/execution/isolate.cc
vendored
|
@ -4133,6 +4133,12 @@ Isolate* Isolate::Allocate(IsolateGroup* group) {
|
|||
|
||||
// static
|
||||
void Isolate::Delete(Isolate* isolate) {
|
||||
Deinitialize(isolate);
|
||||
Free(isolate);
|
||||
}
|
||||
|
||||
// static
|
||||
void Isolate::Deinitialize(Isolate* isolate) {
|
||||
DCHECK_NOT_NULL(isolate);
|
||||
// v8::V8::Dispose() must only be called after deleting all isolates.
|
||||
DCHECK_NOT_NULL(V8::GetCurrentPlatform());
|
||||
|
@ -4156,13 +4162,18 @@ void Isolate::Delete(Isolate* isolate) {
|
|||
isolate->~Isolate();
|
||||
// Only release the group once all other Isolate members have been destroyed.
|
||||
group->Release();
|
||||
// Free the isolate itself.
|
||||
base::AlignedFree(isolate);
|
||||
|
||||
// Restore the previous current isolate.
|
||||
SetIsolateThreadLocals(saved_isolate, saved_data);
|
||||
}
|
||||
|
||||
// static
|
||||
void Isolate::Free(Isolate* isolate) {
|
||||
DCHECK_NOT_NULL(isolate);
|
||||
// Free the memory allocated for the Isolate.
|
||||
base::AlignedFree(isolate);
|
||||
}
|
||||
|
||||
void Isolate::SetUpFromReadOnlyArtifacts(ReadOnlyArtifacts* artifacts) {
|
||||
DCHECK_NOT_NULL(artifacts);
|
||||
InitializeNextUniqueSfiId(artifacts->initial_next_unique_sfi_id());
|
||||
|
|
14
deps/v8/src/execution/isolate.h
vendored
14
deps/v8/src/execution/isolate.h
vendored
|
@ -661,10 +661,16 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
|
|||
static Isolate* New();
|
||||
static Isolate* New(IsolateGroup* isolate_group);
|
||||
|
||||
// Deletes Isolate object. Must be used instead of delete operator.
|
||||
// Destroys the non-default isolates.
|
||||
// Sets default isolate into "has_been_disposed" state rather then destroying,
|
||||
// for legacy API reasons.
|
||||
// Deinitialize Isolate object. Must be used instead of delete operator.
|
||||
// Destroys the non-default isolates. Sets default isolate into
|
||||
// "has_been_disposed" state rather then destroying, for legacy API reasons.
|
||||
// Another Free() call must be done after the isolate is deinitialized.
|
||||
// This gives the embedder a chance to clean up before the address is released
|
||||
// (which maybe reused in the next allocation).
|
||||
static void Deinitialize(Isolate* isolate);
|
||||
// Frees the address of the isolate. Must be called after Deinitialize().
|
||||
static void Free(Isolate* isolate);
|
||||
// A convenience helper that deinitializes and frees the isolate.
|
||||
static void Delete(Isolate* isolate);
|
||||
|
||||
void SetUpFromReadOnlyArtifacts(ReadOnlyArtifacts* artifacts);
|
||||
|
|
19
deps/v8/test/cctest/test-api.cc
vendored
19
deps/v8/test/cctest/test-api.cc
vendored
|
@ -31344,3 +31344,22 @@ TEST(LocalCasts) {
|
|||
v8::MaybeLocal<v8::String>::Cast(no_data);
|
||||
}
|
||||
}
|
||||
|
||||
TEST(IsolateFree) {
|
||||
v8::Isolate* isolate1 = v8::Isolate::Allocate();
|
||||
v8::Isolate::Initialize(isolate1, CreateTestParams());
|
||||
isolate1->Deinitialize();
|
||||
|
||||
// When a new isolate is allocated immediately after one is freed, there is
|
||||
// a chance that the new isolate will be allocated at the same address as the
|
||||
// old one. This tests that when Deinitialize() is called, allocating a new
|
||||
// isolate does not reuse the old address.
|
||||
v8::Isolate* isolate2 = v8::Isolate::Allocate();
|
||||
CHECK_NE(isolate1, isolate2);
|
||||
|
||||
v8::Isolate::Initialize(isolate2, CreateTestParams());
|
||||
isolate2->Deinitialize();
|
||||
|
||||
v8::Isolate::Free(isolate1);
|
||||
v8::Isolate::Free(isolate2);
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue