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

process: refactor global.queueMicrotask() #26523

Closed
wants to merge 1 commit into from
Closed
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
19 changes: 6 additions & 13 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ process.emitWarning = emitWarning;
const {
nextTick,
runNextTicks
} = NativeModule.require('internal/process/next_tick').setup();
} = NativeModule.require('internal/process/task_queues').setupTaskQueue();

process.nextTick = nextTick;
// Used to emulate a tick manually in the JS land.
Expand Down Expand Up @@ -201,7 +201,11 @@ if (!config.noBrowserGlobals) {
defineOperation(global, 'clearTimeout', timers.clearTimeout);
defineOperation(global, 'setInterval', timers.setInterval);
defineOperation(global, 'setTimeout', timers.setTimeout);
setupQueueMicrotask();

const {
queueMicrotask
} = NativeModule.require('internal/process/task_queues');
defineOperation(global, 'queueMicrotask', queueMicrotask);

// Non-standard extensions:
defineOperation(global, 'clearImmediate', timers.clearImmediate);
Expand Down Expand Up @@ -395,17 +399,6 @@ function createGlobalConsole(consoleFromVM) {
return consoleFromNode;
}

function setupQueueMicrotask() {
const { queueMicrotask } =
NativeModule.require('internal/queue_microtask');
Object.defineProperty(global, 'queueMicrotask', {
value: queueMicrotask,
writable: true,
enumerable: true,
configurable: true,
});
}

function setupDOMException() {
// Registers the constructor with C++.
const DOMException = NativeModule.require('internal/domexception');
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ Module.runMain = function() {
return loader.import(pathToFileURL(process.argv[1]).pathname);
})
.catch((e) => {
internalBinding('util').triggerFatalException(e);
internalBinding('task_queue').triggerFatalException(e);
});
} else {
Module._load(process.argv[1], null, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ const {
tickInfo,
// Used to run V8's micro task queue.
runMicrotasks,
setTickCallback
setTickCallback,
enqueueMicrotask,
triggerFatalException
} = internalBinding('task_queue');

const {
Expand All @@ -27,7 +29,10 @@ const {
emitDestroy,
symbols: { async_id_symbol, trigger_async_id_symbol }
} = require('internal/async_hooks');
const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
const {
ERR_INVALID_CALLBACK,
ERR_INVALID_ARG_TYPE
} = require('internal/errors').codes;
const FixedQueue = require('internal/fixed_queue');

// *Must* match Environment::TickInfo::Fields in src/env.h.
Expand Down Expand Up @@ -130,15 +135,52 @@ function nextTick(callback) {
queue.push(new TickObject(callback, args, getDefaultTriggerAsyncId()));
}

// TODO(joyeecheung): make this a factory class so that node.js can
// control the side effects caused by the initializers.
exports.setup = function() {
// Sets the per-isolate promise rejection callback
listenForRejections();
// Sets the callback to be run in every tick.
setTickCallback(processTicksAndRejections);
return {
nextTick,
runNextTicks
};
let AsyncResource;
function createMicrotaskResource() {
// Lazy load the async_hooks module
if (!AsyncResource) {
AsyncResource = require('async_hooks').AsyncResource;
}
return new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});
}

function queueMicrotask(callback) {
if (typeof callback !== 'function') {
throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
}

const asyncResource = createMicrotaskResource();

enqueueMicrotask(() => {
asyncResource.runInAsyncScope(() => {
try {
callback();
} catch (error) {
// TODO(devsnek) remove this if
// https://bugs.chromium.org/p/v8/issues/detail?id=8326
// is resolved such that V8 triggers the fatal exception
// handler for microtasks
triggerFatalException(error);
} finally {
asyncResource.emitDestroy();
}
});
});
}

module.exports = {
setupTaskQueue() {
// Sets the per-isolate promise rejection callback
listenForRejections();
// Sets the callback to be run in every tick.
setTickCallback(processTicksAndRejections);
return {
nextTick,
runNextTicks
};
},
queueMicrotask
};
38 changes: 0 additions & 38 deletions lib/internal/queue_microtask.js

This file was deleted.

3 changes: 1 addition & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -160,16 +160,15 @@
'lib/internal/process/esm_loader.js',
'lib/internal/process/execution.js',
'lib/internal/process/main_thread_only.js',
'lib/internal/process/next_tick.js',
'lib/internal/process/per_thread.js',
'lib/internal/process/policy.js',
'lib/internal/process/promises.js',
'lib/internal/process/stdio.js',
'lib/internal/process/warning.js',
'lib/internal/process/worker_thread_only.js',
'lib/internal/process/report.js',
'lib/internal/process/task_queues.js',
'lib/internal/querystring.js',
'lib/internal/queue_microtask.js',
'lib/internal/readline.js',
'lib/internal/repl.js',
'lib/internal/repl/await.js',
Expand Down
12 changes: 0 additions & 12 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ using errors::TryCatchScope;
using v8::Context;
using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Int32;
using v8::Isolate;
Expand Down Expand Up @@ -752,15 +751,4 @@ void FatalException(Isolate* isolate,
}
}

void FatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr && env->abort_on_uncaught_exception()) {
Abort();
}
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
FatalException(isolate, exception, message);
}

} // namespace node
2 changes: 0 additions & 2 deletions src/node_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ void FatalException(v8::Isolate* isolate,
v8::Local<v8::Value> error,
v8::Local<v8::Message> message);

void FatalException(const v8::FunctionCallbackInfo<v8::Value>& args);

// Helpers to construct errors similar to the ones provided by
// lib/internal/errors.js.
// Example: with `V(ERR_INVALID_ARG_TYPE, TypeError)`, there will be
Expand Down
25 changes: 25 additions & 0 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "env-inl.h"
#include "node.h"
#include "node_errors.h"
#include "node_internals.h"
#include "v8.h"

Expand All @@ -9,6 +10,7 @@ namespace node {

using v8::Array;
using v8::Context;
using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::Isolate;
Expand All @@ -17,6 +19,7 @@ using v8::kPromiseRejectAfterResolved;
using v8::kPromiseRejectWithNoHandler;
using v8::kPromiseResolveAfterResolved;
using v8::Local;
using v8::Message;
using v8::Number;
using v8::Object;
using v8::Promise;
Expand All @@ -26,6 +29,15 @@ using v8::Value;

namespace task_queue {

static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

CHECK(args[0]->IsFunction());

isolate->EnqueueMicrotask(args[0].As<Function>());
}

static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
args.GetIsolate()->RunMicrotasks();
}
Expand Down Expand Up @@ -95,13 +107,26 @@ static void SetPromiseRejectCallback(
env->set_promise_reject_callback(args[0].As<Function>());
}

static void TriggerFatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr && env->abort_on_uncaught_exception()) {
Abort();
}
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
FatalException(isolate, exception, message);
}

static void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();

env->SetMethod(target, "triggerFatalException", TriggerFatalException);
env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
env->SetMethod(target, "setTickCallback", SetTickCallback);
env->SetMethod(target, "runMicrotasks", RunMicrotasks);
target->Set(env->context(),
Expand Down
11 changes: 0 additions & 11 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,6 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
}

void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();

CHECK(args[0]->IsFunction());

isolate->EnqueueMicrotask(args[0].As<Function>());
}

class WeakReference : public BaseObject {
public:
WeakReference(Environment* env, Local<Object> object, Local<Object> target)
Expand Down Expand Up @@ -261,8 +252,6 @@ void Initialize(Local<Object> target,
WatchdogHasPendingSigint);

env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
env->SetMethod(target, "enqueueMicrotask", EnqueueMicrotask);
env->SetMethod(target, "triggerFatalException", FatalException);
Local<Object> constants = Object::New(env->isolate());
NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
Expand Down
4 changes: 2 additions & 2 deletions test/message/events_unhandled_error_nexttick.out
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Error
at internal/main/run_main_module.js:*:*
Emitted 'error' event at:
at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
at processTicksAndRejections (internal/process/next_tick.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
at processTicksAndRejections (internal/process/task_queues.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*
4 changes: 2 additions & 2 deletions test/message/nexttick_throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
^
ReferenceError: undefined_reference_error_maker is not defined
at *test*message*nexttick_throw.js:*:*
at processTicksAndRejections (internal/process/next_tick.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
at processTicksAndRejections (internal/process/task_queues.js:*:*)
at process.runNextTicks [as _tickCallback] (internal/process/task_queues.js:*:*)
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at internal/main/run_main_module.js:*:*