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 f613b30

Browse filesBrowse files
Julien GilliMyles Borins
authored andcommitted
test: add test-domain-exit-dispose-again back
1c85849 "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 preserve the changes made by 1c85849, but it moves them to a new test file named test-timers-reset-process-domain-on-throw.js. PR: #4256 PR-URL: #4256 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
1 parent 39dc054 commit f613b30
Copy full SHA for f613b30

File tree

Expand file treeCollapse file tree

2 files changed

+113
-31
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+113
-31
lines changed
Open diff view settings
Collapse file
+68-31Lines changed: 68 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,76 @@
11
'use strict';
2-
const common = require('../common');
3-
const assert = require('assert');
4-
const domain = require('domain');
52

6-
// Use the same timeout value so that both timers' callbacks are called during
7-
// the same invocation of the underlying native timer's callback (listOnTimeout
8-
// in lib/timers.js).
9-
setTimeout(err, 50);
10-
setTimeout(common.mustCall(secondTimer), 50);
3+
// This test makes sure that when a domain is disposed, timers that are
4+
// attached to that domain are not fired, but timers that are _not_ attached
5+
// to that domain, including those whose callbacks are called from within
6+
// the same invocation of listOnTimeout, _are_ called.
117

12-
function err() {
8+
var common = require('../common');
9+
var assert = require('assert');
10+
var domain = require('domain');
11+
var disposalFailed = false;
12+
13+
// Repeatedly schedule a timer with a delay different than the timers attached
14+
// to a domain that will eventually be disposed to make sure that they are
15+
// called, regardless of what happens with those timers attached to domains
16+
// that will eventually be disposed.
17+
var a = 0;
18+
log();
19+
function log() {
20+
console.log(a++, process.domain);
21+
if (a < 10) setTimeout(log, 20);
22+
}
23+
24+
var secondTimerRan = false;
25+
26+
// Use the same timeout duration for both "firstTimer" and "secondTimer"
27+
// callbacks so that they are called during the same invocation of the
28+
// underlying native timer's callback (listOnTimeout in lib/timers.js).
29+
const TIMEOUT_DURATION = 50;
30+
31+
setTimeout(function firstTimer() {
1332
const d = domain.create();
14-
d.on('error', handleDomainError);
15-
d.run(err2);
1633

17-
function err2() {
18-
// this function doesn't exist, and throws an error as a result.
34+
d.on('error', function handleError(err) {
35+
// Dispose the domain on purpose, so that we can test that nestedTimer
36+
// is not called since it's associated to this domain and a timer whose
37+
// domain is diposed should not run.
38+
d.dispose();
39+
console.error(err);
40+
console.error('in domain error handler',
41+
process.domain, process.domain === d);
42+
});
43+
44+
d.run(function() {
45+
// Create another nested timer that is by definition associated to the
46+
// domain "d". Because an error is thrown before the timer's callback
47+
// is called, and because the domain's error handler disposes the domain,
48+
// this timer's callback should never run.
49+
setTimeout(function nestedTimer() {
50+
console.error('Nested timer should not run, because it is attached to ' +
51+
'a domain that should be disposed.');
52+
disposalFailed = true;
53+
process.exit(1);
54+
});
55+
56+
// Make V8 throw an unreferenced error. As a result, the domain's error
57+
// handler is called, which disposes the domain "d" and should prevent the
58+
// nested timer that is attached to it from running.
1959
err3();
20-
}
60+
});
61+
}, TIMEOUT_DURATION);
2162

22-
function handleDomainError(e) {
23-
// In the domain's error handler, the current active domain should be the
24-
// domain within which the error was thrown.
25-
assert.equal(process.domain, d);
26-
}
27-
}
63+
// This timer expires in the same invocation of listOnTimeout than firstTimer,
64+
// but because it's not attached to any domain, it must run regardless of
65+
// domain "d" being disposed.
66+
setTimeout(function secondTimer() {
67+
console.log('In second timer');
68+
secondTimerRan = true;
69+
}, TIMEOUT_DURATION);
2870

29-
function secondTimer() {
30-
// secondTimer was scheduled before any domain had been created, so its
31-
// callback should not have any active domain set when it runs.
32-
// Do not use assert here, as it throws errors and if a domain with an error
33-
// handler is active, then asserting wouldn't make the test fail.
34-
if (process.domain !== null) {
35-
console.log('process.domain should be null, but instead is:',
36-
process.domain);
37-
process.exit(1);
38-
}
39-
}
71+
process.on('exit', function() {
72+
assert.equal(a, 10);
73+
assert.equal(disposalFailed, false);
74+
assert(secondTimerRan);
75+
console.log('ok');
76+
});
Collapse file
+45Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
// This test makes sure that when throwing from within a timer's callback,
4+
// its active domain at the time of the throw is not the process' active domain
5+
// for the next timers that need to be processed on the same turn of the event
6+
// loop.
7+
8+
const common = require('../common');
9+
const assert = require('assert');
10+
const domain = require('domain');
11+
12+
// Use the same timeout value so that both timers' callbacks are called during
13+
// the same invocation of the underlying native timer's callback (listOnTimeout
14+
// in lib/timers.js).
15+
setTimeout(err, 50);
16+
setTimeout(common.mustCall(secondTimer), 50);
17+
18+
function err() {
19+
const d = domain.create();
20+
d.on('error', handleDomainError);
21+
d.run(err2);
22+
23+
function err2() {
24+
// this function doesn't exist, and throws an error as a result.
25+
err3();
26+
}
27+
28+
function handleDomainError(e) {
29+
// In the domain's error handler, the current active domain should be the
30+
// domain within which the error was thrown.
31+
assert.equal(process.domain, d);
32+
}
33+
}
34+
35+
function secondTimer() {
36+
// secondTimer was scheduled before any domain had been created, so its
37+
// callback should not have any active domain set when it runs.
38+
if (process.domain !== null) {
39+
console.log('process.domain should be null in this timer callback, but ' +
40+
'instead is:', process.domain);
41+
// Do not use assert here, as it throws errors and if a domain with an error
42+
// handler is active, then asserting wouldn't make the test fail.
43+
process.exit(1);
44+
}
45+
}

0 commit comments

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