Skip to content

Commit

Permalink
src: implement v8 host weakref hooks
Browse files Browse the repository at this point in the history
PR-URL: #29874
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
devsnek authored and MylesBorins committed Jan 8, 2020
1 parent 512a0cc commit 17e0bf4
Show file tree
Hide file tree
Showing 11 changed files with 94 additions and 0 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,5 +324,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 @@ -28,6 +28,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 @@ -1050,6 +1051,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());
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 @@ -1132,6 +1132,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 @@ -1338,6 +1341,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);

0 comments on commit 17e0bf4

Please sign in to comment.