diff options
author | Gus Caplan <me@gus.host> | 2020-05-12 13:26:38 -0500 |
---|---|---|
committer | Gus Caplan <me@gus.host> | 2020-05-13 19:12:11 -0500 |
commit | eaa16cd477f671804a46b57bfdfe918a02334f4d (patch) | |
tree | 1cefef9d2b68fe4fc97361a0d925ddbfff853c64 | |
parent | fcc183c99413750b1b965e45cb42a1af73da47ab (diff) | |
download | node-new-eaa16cd477f671804a46b57bfdfe918a02334f4d.tar.gz |
src: remove deprecated FinalizationRegistry hooks
PR-URL: https://github.com/nodejs/node/pull/33373
Fixes: https://github.com/nodejs/node/issues/33389
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
-rw-r--r-- | src/api/environment.cc | 15 | ||||
-rw-r--r-- | src/env-inl.h | 7 | ||||
-rw-r--r-- | src/env.cc | 22 | ||||
-rw-r--r-- | src/env.h | 4 | ||||
-rw-r--r-- | src/node.h | 2 | ||||
-rw-r--r-- | test/parallel/test-finalization-group-error.js | 27 | ||||
-rw-r--r-- | test/parallel/test-finalization-group-regular-gc.js | 25 | ||||
-rw-r--r-- | test/parallel/test-finalization-group.js | 13 |
8 files changed, 0 insertions, 115 deletions
diff --git a/src/api/environment.cc b/src/api/environment.cc index b9ca6ca745..2b7a7023b5 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -16,7 +16,6 @@ using errors::TryCatchScope; using v8::Array; using v8::Context; using v8::EscapableHandleScope; -using v8::FinalizationGroup; using v8::Function; using v8::FunctionCallbackInfo; using v8::HandleScope; @@ -82,15 +81,6 @@ static MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context, return result; } -static void HostCleanupFinalizationGroupCallback( - Local<Context> context, Local<FinalizationGroup> group) { - Environment* env = Environment::GetCurrent(context); - if (env == nullptr) { - return; - } - env->RegisterFinalizationGroupForCleanup(group); -} - void* NodeArrayBufferAllocator::Allocate(size_t size) { void* ret; if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers) @@ -259,11 +249,6 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) { s.promise_reject_callback : task_queue::PromiseRejectCallback; isolate->SetPromiseRejectCallback(promise_reject_cb); - auto* host_cleanup_cb = s.host_cleanup_finalization_group_callback ? - s.host_cleanup_finalization_group_callback : - HostCleanupFinalizationGroupCallback; - isolate->SetHostCleanupFinalizationGroupCallback(host_cleanup_cb); - if (s.flags & DETAILED_SOURCE_POSITIONS_FOR_PROFILING) v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate); } diff --git a/src/env-inl.h b/src/env-inl.h index a2db24ed17..b5a044a1b0 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -1120,13 +1120,6 @@ void Environment::RemoveCleanupHook(void (*fn)(void*), void* arg) { cleanup_hooks_.erase(search); } -inline void Environment::RegisterFinalizationGroupForCleanup( - v8::Local<v8::FinalizationGroup> group) { - cleanup_finalization_groups_.emplace_back(isolate(), group); - DCHECK(task_queues_async_initialized_); - uv_async_send(&task_queues_async_); -} - size_t CleanupHookCallback::Hash::operator()( const CleanupHookCallback& cb) const { return std::hash<void*>()(cb.arg_); diff --git a/src/env.cc b/src/env.cc index 06e6fe6f79..b83bd62835 100644 --- a/src/env.cc +++ b/src/env.cc @@ -31,7 +31,6 @@ using v8::ArrayBuffer; using v8::Boolean; using v8::Context; using v8::EmbedderGraph; -using v8::FinalizationGroup; using v8::Function; using v8::FunctionTemplate; using v8::HandleScope; @@ -523,7 +522,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { [](uv_async_t* async) { Environment* env = ContainerOf( &Environment::task_queues_async_, async); - env->CleanupFinalizationGroups(); env->RunAndClearNativeImmediates(); }); uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_)); @@ -1158,26 +1156,6 @@ void Environment::RunWeakRefCleanup() { isolate()->ClearKeptObjects(); } -void Environment::CleanupFinalizationGroups() { - HandleScope handle_scope(isolate()); - Context::Scope context_scope(context()); - TryCatchScope try_catch(this); - - while (!cleanup_finalization_groups_.empty() && can_call_into_js()) { - Local<FinalizationGroup> fg = - cleanup_finalization_groups_.front().Get(isolate()); - cleanup_finalization_groups_.pop_front(); - if (!FinalizationGroup::Cleanup(fg).FromMaybe(false)) { - if (try_catch.HasCaught() && !try_catch.HasTerminated()) - errors::TriggerUncaughtException(isolate(), try_catch); - // Re-schedule the execution of the remainder of the queue. - CHECK(task_queues_async_initialized_); - uv_async_send(&task_queues_async_); - return; - } - } -} - // Not really any better place than env.cc at this moment. void BaseObject::DeleteMe(void* data) { BaseObject* self = static_cast<BaseObject*>(data); @@ -1107,9 +1107,7 @@ class Environment : public MemoryRetainer { void AtExit(void (*cb)(void* arg), void* arg); void RunAtExitCallbacks(); - void RegisterFinalizationGroupForCleanup(v8::Local<v8::FinalizationGroup> fg); void RunWeakRefCleanup(); - void CleanupFinalizationGroups(); // Strings and private symbols are shared across shared contexts // The getters simply proxy to the per-isolate primitive. @@ -1334,8 +1332,6 @@ class Environment : public MemoryRetainer { uint64_t thread_id_; std::unordered_set<worker::Worker*> sub_worker_contexts_; - std::deque<v8::Global<v8::FinalizationGroup>> cleanup_finalization_groups_; - static void* const kNodeContextTagPtr; static int const kNodeContextTag; diff --git a/src/node.h b/src/node.h index c97b425655..72989617f7 100644 --- a/src/node.h +++ b/src/node.h @@ -346,8 +346,6 @@ struct IsolateSettings { v8::PromiseRejectCallback promise_reject_callback = nullptr; v8::AllowWasmCodeGenerationCallback allow_wasm_code_generation_callback = nullptr; - v8::HostCleanupFinalizationGroupCallback - host_cleanup_finalization_group_callback = nullptr; }; // Overriding IsolateSettings may produce unexpected behavior diff --git a/test/parallel/test-finalization-group-error.js b/test/parallel/test-finalization-group-error.js deleted file mode 100644 index 46a670073b..0000000000 --- a/test/parallel/test-finalization-group-error.js +++ /dev/null @@ -1,27 +0,0 @@ -'use strict'; - -// Flags: --expose-gc --harmony-weak-refs - -const common = require('../common'); -const assert = require('assert'); - -const g = new globalThis.FinalizationRegistry(common.mustCallAtLeast(() => { - throw new Error('test'); -}, 1)); -g.register({}, 42); - -setTimeout(() => { - globalThis.gc(); - assert.throws(() => { - g.cleanupSome(); - }, { - name: 'Error', - message: 'test', - }); - - // Give the callbacks scheduled by global.gc() time to run, as the underlying - // uv_async_t is unref’ed. - setTimeout(() => {}, 200); -}, 200); - -process.on('uncaughtException', common.mustCall()); diff --git a/test/parallel/test-finalization-group-regular-gc.js b/test/parallel/test-finalization-group-regular-gc.js deleted file mode 100644 index 3c16cfcee2..0000000000 --- a/test/parallel/test-finalization-group-regular-gc.js +++ /dev/null @@ -1,25 +0,0 @@ -// Flags: --harmony-weak-refs -'use strict'; -require('../common'); -const assert = require('assert'); - -// Test that finalization callbacks do not crash when caused through a regular -// GC (not global.gc()). - -const start = Date.now(); -const g = new globalThis.FinalizationRegistry(() => { - const diff = Date.now() - start; - assert(diff < 10000, `${diff} >= 10000`); -}); -g.register({}, 42); - -setImmediate(() => { - const arr = []; - // Build up enough memory usage to hopefully trigger a platform task but not - // enough to trigger GC as an interrupt. - while (arr.length < 1000000) arr.push([]); - - setTimeout(() => { - g; // Keep reference alive. - }, 200000).unref(); -}); diff --git a/test/parallel/test-finalization-group.js b/test/parallel/test-finalization-group.js deleted file mode 100644 index 4b9357e4d1..0000000000 --- a/test/parallel/test-finalization-group.js +++ /dev/null @@ -1,13 +0,0 @@ -'use strict'; - -// Flags: --expose-gc --harmony-weak-refs - -const common = require('../common'); - -const g = new globalThis.FinalizationRegistry(common.mustCallAtLeast(1)); -g.register({}, 42); - -setTimeout(() => { - globalThis.gc(); - g.cleanupSome(); -}, 200); |