Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 1effbd7

Browse filesBrowse files
author
Julien Gilli
committed
test: add test-domain-exit-dispose-again back
d1ba82a "fixed" test-domain-exit-dispose-again by changing its logic to test that process.domain was cleared properly in case an error was thrown from a timer's callback. However, it became clear when reviewing a recent change that refactors lib/timers.js that it was not quite the intention of the original test. Thus, this change adds the original implementation of test-domain-exit-dispose-again back, with comments that make its implementation easier to understand. It also preserves the changes made by d1ba82a, but it moves them to a new test file named test-timers-reset-process-domain-on-throw.js. PR: #4278 PR-URL: #4278 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 801f6ad commit 1effbd7
Copy full SHA for 1effbd7

File tree

Expand file treeCollapse file tree

2 files changed

+121
-29
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+121
-29
lines changed
Open diff view settings
Collapse file

‎test/simple/test-domain-exit-dispose-again.js‎

Copy file name to clipboardExpand all lines: test/simple/test-domain-exit-dispose-again.js
+62-29Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -19,41 +19,74 @@
1919
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

22-
var common = require('../common');
22+
// This test makes sure that when a domain is disposed, timers that are
23+
// attached to that domain are not fired, but timers that are _not_ attached
24+
// to that domain, including those whose callbacks are called from within
25+
// the same invocation of listOnTimeout, _are_ called.
26+
27+
var common = require('../common.js');
2328
var assert = require('assert');
2429
var domain = require('domain');
30+
var disposalFailed = false;
31+
32+
// Repeatedly schedule a timer with a delay different than the timers attached
33+
// to a domain that will eventually be disposed to make sure that they are
34+
// called, regardless of what happens with those timers attached to domains
35+
// that will eventually be disposed.
36+
var a = 0;
37+
log();
38+
function log(){
39+
console.log(a++, process.domain);
40+
if (a < 10) setTimeout(log, 20);
41+
}
2542

26-
// Use the same timeout value so that both timers' callbacks are called during
27-
// the same invocation of the underlying native timer's callback (listOnTimeout
28-
// in lib/timers.js).
29-
setTimeout(err, 50);
30-
setTimeout(common.mustCall(secondTimer), 50);
43+
var secondTimerRan = false;
3144

32-
function err() {
45+
// Use the same timeout duration for both "firstTimer" and "secondTimer"
46+
// callbacks so that they are called during the same invocation of the
47+
// underlying native timer's callback (listOnTimeout in lib/timers.js).
48+
var TIMEOUT_DURATION = 50;
49+
50+
setTimeout(function firstTimer() {
3351
var d = domain.create();
34-
d.on('error', handleDomainError);
35-
d.run(err2);
52+
d.on('error', function handleError(err) {
53+
// Dispose the domain on purpose, so that we can test that nestedTimer
54+
// is not called since it's associated to this domain and a timer whose
55+
// domain is diposed should not run.
56+
d.dispose();
57+
console.error(err);
58+
console.error('in handler', process.domain, process.domain === d);
59+
});
60+
d.run(function() {
61+
// Create another nested timer that is by definition associated to the
62+
// domain "d". Because an error is thrown before the timer's callback
63+
// is called, and because the domain's error handler disposes the domain,
64+
// this timer's callback should never run.
65+
setTimeout(function nestedTimer() {
66+
console.error('Nested timer should not run, because it is attached to ' +
67+
'a domain that should be disposed.');
68+
disposalFailed = true;
69+
process.exit(1);
70+
});
3671

37-
function err2() {
38-
// this function doesn't exist, and throws an error as a result.
72+
// Make V8 throw an unreferenced error. As a result, the domain's error
73+
// handler is called, which disposes the domain "d" and should prevent the
74+
// nested timer that is attached to it from running.
3975
err3();
40-
}
76+
});
77+
}, TIMEOUT_DURATION);
4178

42-
function handleDomainError(e) {
43-
// In the domain's error handler, the current active domain should be the
44-
// domain within which the error was thrown.
45-
assert.equal(process.domain, d);
46-
}
47-
}
79+
// This timer expires in the same invocation of listOnTimeout than firstTimer,
80+
// but because it's not attached to any domain, it must run regardless of
81+
// domain "d" being disposed.
82+
setTimeout(function secondTimer() {
83+
console.log('In second timer');
84+
secondTimerRan = true;
85+
}, TIMEOUT_DURATION);
4886

49-
function secondTimer() {
50-
// secondTimer was scheduled before any domain had been created, so its
51-
// callback should not have any active domain set when it runs.
52-
// Do not use assert here, as it throws errors and if a domain with an error
53-
// handler is active, then asserting wouldn't make the test fail.
54-
if (process.domain !== null) {
55-
console.log('process.domain should be null, but instead is:',
56-
process.domain);
57-
process.exit(1);
58-
}
59-
}
87+
process.on('exit', function() {
88+
assert.equal(a, 10);
89+
assert.equal(disposalFailed, false);
90+
assert(secondTimerRan);
91+
console.log('ok');
92+
});
Collapse file
+59Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
var domain = require('domain');
25+
26+
// Use the same timeout value so that both timers' callbacks are called during
27+
// the same invocation of the underlying native timer's callback (listOnTimeout
28+
// in lib/timers.js).
29+
setTimeout(err, 50);
30+
setTimeout(common.mustCall(secondTimer), 50);
31+
32+
function err() {
33+
var d = domain.create();
34+
d.on('error', handleDomainError);
35+
d.run(err2);
36+
37+
function err2() {
38+
// this function doesn't exist, and throws an error as a result.
39+
err3();
40+
}
41+
42+
function handleDomainError(e) {
43+
// In the domain's error handler, the current active domain should be the
44+
// domain within which the error was thrown.
45+
assert.equal(process.domain, d);
46+
}
47+
}
48+
49+
function secondTimer() {
50+
// secondTimer was scheduled before any domain had been created, so its
51+
// callback should not have any active domain set when it runs.
52+
// Do not use assert here, as it throws errors and if a domain with an error
53+
// handler is active, then asserting wouldn't make the test fail.
54+
if (process.domain !== null) {
55+
console.log('process.domain should be null, but instead is:',
56+
process.domain);
57+
process.exit(1);
58+
}
59+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.