Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: implement v8 host weakref hooks #29874

Merged
merged 1 commit into from
Oct 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,5 +317,6 @@ module.exports = {
TextEncoder: 'readable',
TextDecoder: 'readable',
queueMicrotask: 'readable',
globalThis: 'readable',
},
};
3 changes: 3 additions & 0 deletions src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ void InternalCallbackScope::Close() {
TickInfo* tick_info = env_->tick_info();

if (!env_->can_call_into_js()) return;

OnScopeLeave weakref_cleanup([&]() { env_->RunWeakRefCleanup(); });

if (!tick_info->has_tick_scheduled()) {
MicrotasksScope::PerformCheckpoint(env_->isolate());
}
Expand Down
12 changes: 12 additions & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ using errors::TryCatchScope;
using v8::Array;
using v8::Context;
using v8::EscapableHandleScope;
using v8::FinalizationGroup;
using v8::Function;
using v8::HandleScope;
using v8::Isolate;
Expand Down Expand Up @@ -76,6 +77,15 @@ 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) {
if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
return UncheckedCalloc(size);
Expand Down Expand Up @@ -203,6 +213,8 @@ void SetIsolateUpForNode(v8::Isolate* isolate, IsolateSettingCategories cat) {
isolate->SetAllowWasmCodeGenerationCallback(
AllowWasmCodeGenerationCallback);
isolate->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
isolate->SetHostCleanupFinalizationGroupCallback(
HostCleanupFinalizationGroupCallback);
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
break;
default:
Expand Down
5 changes: 5 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1115,6 +1115,11 @@ 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);
}

size_t CleanupHookCallback::Hash::operator()(
const CleanupHookCallback& cb) const {
return std::hash<void*>()(cb.arg_);
Expand Down
16 changes: 16 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ using v8::ArrayBuffer;
using v8::Boolean;
using v8::Context;
using v8::EmbedderGraph;
using v8::FinalizationGroup;
using v8::Function;
using v8::FunctionTemplate;
using v8::HandleScope;
Expand Down Expand Up @@ -1051,6 +1052,21 @@ void Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
keep_alive_allocators_->insert(allocator);
}

bool Environment::RunWeakRefCleanup() {
isolate()->ClearKeptObjects();

while (!cleanup_finalization_groups_.empty()) {
Local<FinalizationGroup> fg =
cleanup_finalization_groups_.front().Get(isolate());
devsnek marked this conversation as resolved.
Show resolved Hide resolved
cleanup_finalization_groups_.pop_front();
if (!FinalizationGroup::Cleanup(fg).FromMaybe(false)) {
return false;
}
}

return true;
}

void AsyncRequest::Install(Environment* env, void* data, uv_async_cb target) {
CHECK_NULL(async_);
env_ = env;
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,9 @@ class Environment : public MemoryRetainer {
void AtExit(void (*cb)(void* arg), void* arg);
void RunAtExitCallbacks();

void RegisterFinalizationGroupForCleanup(v8::Local<v8::FinalizationGroup> fg);
bool RunWeakRefCleanup();

// Strings and private symbols are shared across shared contexts
// The getters simply proxy to the per-isolate primitive.
#define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
Expand Down Expand Up @@ -1335,6 +1338,8 @@ 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;

Expand Down
2 changes: 2 additions & 0 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {

// Should be in sync with runNextTicks in internal/process/task_queues.js
bool RunNextTicksNative(Environment* env) {
OnScopeLeave weakref_cleanup([&]() { env->RunWeakRefCleanup(); });

TickInfo* tick_info = env->tick_info();
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
MicrotasksScope::PerformCheckpoint(env->isolate());
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ globals:
BigInt64Array: false
BigUint64Array: false
SharedArrayBuffer: false
globalThis: false
23 changes: 23 additions & 0 deletions test/parallel/test-finalization-group-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';

// Flags: --expose-gc --harmony-weak-refs

const common = require('../common');
const assert = require('assert');

const g = new globalThis.FinalizationGroup(common.mustCallAtLeast(() => {
throw new Error('test');
}, 1));
g.register({}, 42);

setTimeout(() => {
globalThis.gc();
assert.throws(() => {
g.cleanupSome();
}, {
name: 'Error',
message: 'test',
});
}, 200);

process.on('uncaughtException', common.mustCall());
13 changes: 13 additions & 0 deletions test/parallel/test-finalization-group.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

// Flags: --expose-gc --harmony-weak-refs

const common = require('../common');

const g = new globalThis.FinalizationGroup(common.mustCallAtLeast(1));
g.register({}, 42);

setTimeout(() => {
globalThis.gc();
g.cleanupSome();
}, 200);
13 changes: 13 additions & 0 deletions test/parallel/test-weakref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

// Flags: --expose-gc --harmony-weak-refs

require('../common');
const assert = require('assert');

const w = new globalThis.WeakRef({});

setTimeout(() => {
globalThis.gc();
assert.strictEqual(w.deref(), undefined);
}, 200);