Skip to content

Commit

Permalink
process: make internal/queue_microtask.js more self-contained
Browse files Browse the repository at this point in the history
- Instead of passing triggerFatalException through node.js,
  simply put it on `internalBinding('util')` which has to be
  loaded anyway.
- Expose the implementation of `queueMicrotask` directly instead
  of through an unnecessary factory function.

PR-URL: nodejs#25189
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
joyeecheung authored and refack committed Jan 10, 2019
1 parent 18f60b1 commit eafd8a6
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 41 deletions.
11 changes: 5 additions & 6 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

// This file is compiled as if it's wrapped in a function with arguments
// passed by node::LoadEnvironment()
/* global process, loaderExports, triggerFatalException */
/* global isMainThread */
/* global process, loaderExports, isMainThread */

const { internalBinding, NativeModule } = loaderExports;

Expand Down Expand Up @@ -605,12 +604,12 @@ function setupGlobalEncoding() {

function setupQueueMicrotask() {
Object.defineProperty(global, 'queueMicrotask', {
get: () => {
get() {
process.emitWarning('queueMicrotask() is experimental.',
'ExperimentalWarning');
const { setupQueueMicrotask } =
const { queueMicrotask } =
NativeModule.require('internal/queue_microtask');
const queueMicrotask = setupQueueMicrotask(triggerFatalException);

Object.defineProperty(global, 'queueMicrotask', {
value: queueMicrotask,
writable: true,
Expand All @@ -619,7 +618,7 @@ function setupQueueMicrotask() {
});
return queueMicrotask;
},
set: (v) => {
set(v) {
Object.defineProperty(global, 'queueMicrotask', {
value: v,
writable: true,
Expand Down
57 changes: 28 additions & 29 deletions lib/internal/queue_microtask.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,36 @@
const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
const { AsyncResource } = require('async_hooks');
const { getDefaultTriggerAsyncId } = require('internal/async_hooks');
const { enqueueMicrotask } = internalBinding('util');
const {
enqueueMicrotask,
triggerFatalException
} = internalBinding('util');

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

const asyncResource = new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});
const asyncResource = new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});

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();
}
});
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();
}
});
};

return queueMicrotask;
};
});
}

module.exports = { setupQueueMicrotask };
module.exports = { queueMicrotask };
6 changes: 1 addition & 5 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1200,18 +1200,14 @@ void LoadEnvironment(Environment* env) {
return;
}

// process, bootstrappers, loaderExports, triggerFatalException
// process, loaderExports, isMainThread
std::vector<Local<String>> node_params = {
env->process_string(),
FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
FIXED_ONE_BYTE_STRING(isolate, "triggerFatalException"),
FIXED_ONE_BYTE_STRING(isolate, "isMainThread")};
std::vector<Local<Value>> node_args = {
process,
loader_exports.ToLocalChecked(),
env->NewFunctionTemplate(FatalException)
->GetFunction(context)
.ToLocalChecked(),
Boolean::New(isolate, env->is_main_thread())};

if (ExecuteBootstrapper(
Expand Down
3 changes: 2 additions & 1 deletion src/node_util.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "node_internals.h"
#include "node_errors.h"
#include "node_watchdog.h"

namespace node {
Expand Down Expand Up @@ -221,7 +222,7 @@ void Initialize(Local<Object> target,
WatchdogHasPendingSigint);

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

0 comments on commit eafd8a6

Please sign in to comment.