Skip to content

Commit f613b30

Browse 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

2 files changed

Lines changed: 113 additions & 31 deletions

File tree

Lines changed: 68 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,76 @@
11
'use strict';
2-
constcommon=require('../common');
3-
constassert=require('assert');
4-
constdomain=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-
functionerr(){
8+
varcommon=require('../common');
9+
varassert=require('assert');
10+
vardomain=require('domain');
11+
vardisposalFailed=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+
vara=0;
18+
log();
19+
functionlog(){
20+
console.log(a++,process.domain);
21+
if(a<10)setTimeout(log,20);
22+
}
23+
24+
varsecondTimerRan=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+
constTIMEOUT_DURATION=50;
30+
31+
setTimeout(functionfirstTimer(){
1332
constd=domain.create();
14-
d.on('error',handleDomainError);
15-
d.run(err2);
1633

17-
functionerr2(){
18-
// this function doesn't exist, and throws an error as a result.
34+
d.on('error',functionhandleError(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(functionnestedTimer(){
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-
functionhandleDomainError(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(functionsecondTimer(){
67+
console.log('In second timer');
68+
secondTimerRan=true;
69+
},TIMEOUT_DURATION);
2870

29-
functionsecondTimer(){
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+
});
Lines 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+
constcommon=require('../common');
9+
constassert=require('assert');
10+
constdomain=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+
functionerr(){
19+
constd=domain.create();
20+
d.on('error',handleDomainError);
21+
d.run(err2);
22+
23+
functionerr2(){
24+
// this function doesn't exist, and throws an error as a result.
25+
err3();
26+
}
27+
28+
functionhandleDomainError(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+
functionsecondTimer(){
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)