Skip to content

Commit

Permalink
async_wrap: return undefined if domain is disposed
Browse files Browse the repository at this point in the history
v8::MaybeLocal::ToLocalChecked() will abort on an empty handle.
AsyncWrap::MakeCallback returns an empty handle if the domain is
disposed, but this should not cause the process to abort. So instead
return v8::Undefined() and allow the error to be handled normally.

PR-URL: #14722
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
trevnorris authored and MylesBorins committed Sep 12, 2017
1 parent 4bf0d4e commit fadccba
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ using v8::String;
using v8::Symbol;
using v8::TryCatch;
using v8::Uint32Array;
using v8::Undefined;
using v8::Value;

using AsyncHooks = node::Environment::AsyncHooks;
Expand Down Expand Up @@ -696,7 +697,7 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
get_trigger_id());

if (!PreCallbackExecution(this, true)) {
return MaybeLocal<Value>();
return Undefined(env()->isolate());
}

// Finally... Get to running the user's callback.
Expand All @@ -707,7 +708,7 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
}

if (!PostCallbackExecution(this, true)) {
return Local<Value>();
return Undefined(env()->isolate());
}

exec_scope.Dispose();
Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-domain-abort-when-disposed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';

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

// These were picked arbitrarily and are only used to trigger arync_hooks.
const JSStream = process.binding('js_stream').JSStream;
const Socket = require('net').Socket;

const handle = new JSStream();
handle.domain = domain.create();
handle.domain.dispose();

handle.close = () => {};
handle.isAlive = () => { throw new Error(); };

const s = new Socket({ handle });

// When AsyncWrap::MakeCallback() returned an empty handle the
// MaybeLocal::ToLocalChecked() call caused the process to abort. By returning
// v8::Undefined() it allows the error to propagate to the 'error' event.
s.on('error', common.mustCall((e) => {
assert.strictEqual(e.code, 'EINVAL');
}));

0 comments on commit fadccba

Please sign in to comment.