Skip to content

Commit 7bd2e80

Browse files
committed
merge from master to fix tests
2 parents 6bd9eeb + 875ec67 commit 7bd2e80

11 files changed

+113
-134
lines changed

.travis.yml

+2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
language: node_js
22
node_js:
33
- "0.10"
4+
- "0.12"
5+
- "iojs"
46

cluster-service.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ if (
7474
// load the worker if not already loaded
7575
// async, in case worker loads cluster-service, we need to return before
7676
// it's avail
77-
setTimeout(function() {
77+
setImmediate(function() {
7878
cluster.worker.module = require(process.env.worker);
7979
if (global.cservice.locals.workerReady === undefined
8080
&& process.env.ready.toString() === "false") {
@@ -84,7 +84,7 @@ if (
8484
exports.workerReady(); // NOW we're ready
8585
});
8686
}
87-
}, 10);
87+
});
8888

8989
// start worker monitor to establish two-way relationship with master
9090
workers.monitor();

lib/commands/restart.js

+21-27
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,7 @@ module.exports.more = function(cb) {
6767

6868
function getTask(evt, worker, options, explicitRestart) {
6969
return function(cb) {
70-
// kill new worker if takes too long
71-
var newKiller = null;
72-
var newWorker = null;
73-
var exitListener = function() {
74-
if (newKiller) {
75-
clearTimeout(newKiller);
76-
}
77-
};
78-
var w;
70+
var pendingWorker = null;
7971

8072
if (worker.cservice.restart === false && explicitRestart === false) {
8173
cservice.log(
@@ -85,42 +77,44 @@ function getTask(evt, worker, options, explicitRestart) {
8577
return;
8678
}
8779

80+
// kill new worker if takes too long
81+
var newWorkerTimeout = null;
82+
var isNewWorkerTerminated = false;
8883
if (options.timeout > 0) { // start timeout if specified
89-
newKiller = setTimeout(function() {
90-
w = newWorker;
91-
newWorker = null;
92-
if (w) {
93-
w.removeListener("exit", exitListener); // remove temp listener
94-
w.kill("SIGKILL"); // go get'em, killer
84+
newWorkerTimeout = setTimeout(function() {
85+
if (pendingWorker) {
86+
isNewWorkerTerminated = true;
87+
pendingWorker.on('exit', function () {
88+
cb("timed out");
89+
});
90+
pendingWorker.kill("SIGKILL"); // go get'em, killer
9591
}
96-
cb("timed out");
9792
}, options.timeout);
9893
}
9994

10095
// lets start new worker
101-
newWorker = evt.service.newWorker(worker.cservice, function(err, newWorker){
102-
var killer;
103-
newWorker.removeListener("exit", exitListener); // remove temp listener
104-
newWorker = null;
105-
if (newKiller) { // timeout no longer needed
106-
clearTimeout(newKiller);
96+
pendingWorker = evt.service.newWorker(worker.cservice, function(err) {
97+
pendingWorker = null;
98+
if (newWorkerTimeout) { // timeout no longer needed
99+
clearTimeout(newWorkerTimeout);
107100
}
101+
if (isNewWorkerTerminated) return;
108102

109103
// ok, lets stop old worker
110-
killer = null;
104+
var oldWorkerTimeout = null;
111105
if (options.timeout > 0) { // start timeout if specified
112-
killer = setTimeout(function() {
106+
oldWorkerTimeout = setTimeout(function() {
113107
worker.kill("SIGKILL"); // go get'em, killer
114108
}, options.timeout);
115109
}
116110

117111
worker.on("exit", function() {
118-
if (killer) {
119-
clearTimeout(killer);
112+
if (oldWorkerTimeout) {
113+
clearTimeout(oldWorkerTimeout);
120114
}
121115

122116
// exit complete, fire callback
123-
setTimeout(cb, 100); // slight delay in case other events are piled up
117+
setImmediate(cb); // slight delay in case other events are piled up
124118
});
125119

126120
require("../workers").exitGracefully(worker);

lib/commands/shutdown.js

+16-24
Original file line numberDiff line numberDiff line change
@@ -28,29 +28,22 @@ module.exports = function(evt, cb, cmd, options) {
2828
exiting = true;
2929
workersToKill++;
3030

31-
worker.process.on(
32-
"exit",
33-
getExitHandler(
34-
evt
35-
, worker
36-
, options.timeout > 0
37-
? setTimeout(getKiller(worker), options.timeout)
38-
: null
39-
, function() {
40-
workersToKill--;
41-
if (workersToKill === 0) {
42-
// no workers remain
43-
if (evt.service.workers.length === 0) {
44-
evt.locals.reason = "kill";
45-
cservice.log("All workers shutdown. Exiting...".warn);
46-
evt.service.stop(options.timeout, cb);
47-
} else {
48-
cb(null, "Worker shutdown"); // DONE
49-
}
50-
}
31+
var killTimeout = options.timeout > 0
32+
? setTimeout(getKiller(worker), options.timeout)
33+
: null;
34+
worker.on("exit", getExitHandler(evt, worker, killTimeout, function() {
35+
workersToKill--;
36+
if (workersToKill === 0) {
37+
// no workers remain
38+
if (evt.service.workers.length === 0) {
39+
evt.locals.reason = "kill";
40+
cservice.log("All workers shutdown. Exiting...".warn);
41+
evt.service.stop(options.timeout, cb);
42+
} else {
43+
cb(null, "Worker shutdown"); // DONE
5144
}
52-
)
53-
);
45+
}
46+
}));
5447

5548
require("../workers").exitGracefully(worker);
5649
});
@@ -104,9 +97,8 @@ function getExitHandler(evt, worker, killer, cb) {
10497
return function() {
10598
if (killer) {
10699
clearTimeout(killer);
107-
killer = null;
108100
}
109101

110102
cb();
111103
};
112-
}
104+
}

lib/commands/start.js

+20-23
Original file line numberDiff line numberDiff line change
@@ -64,38 +64,35 @@ module.exports.more = function(cb) {
6464

6565
function getTask(evt, options) {
6666
return function(cb) {
67-
// kill new worker if takes too long
68-
var newKiller = null;
69-
var newWorker;
70-
var exitListener = function() {
71-
if (newKiller) {
72-
clearTimeout(newKiller);
73-
}
74-
};
67+
var pendingWorker;
7568

69+
// kill new worker if takes too long
70+
var startTimeout = null;
71+
var isWorkerTerminated = false;
7672
if (options.timeout > 0) { // start timeout if specified
77-
newKiller = setTimeout(function() {
78-
if (!newWorker)
73+
startTimeout = setTimeout(function() {
74+
if (!pendingWorker)
7975
return;
80-
newWorker.removeListener("exit", exitListener); // remove temp listener
81-
newWorker.kill("SIGKILL"); // go get'em, killer
82-
cb("timed out");
76+
isWorkerTerminated = true;
77+
pendingWorker.on('exit', function () {
78+
cb("timed out");
79+
});
80+
pendingWorker.kill("SIGKILL"); // go get'em, killer
8381
}, options.timeout);
84-
newKiller.unref();
82+
startTimeout.unref();
8583
}
8684

8785
// lets start new worker
88-
newWorker = evt.service.newWorker(options, function(err, newWorker) {
89-
if (newWorker) { // won't exist if failure
90-
newWorker.removeListener("exit", exitListener); // remove temp listener
91-
}
92-
if (newKiller) { // timeout no longer needed
93-
clearTimeout(newKiller);
94-
newKiller = null;
95-
}
86+
pendingWorker = evt.service.newWorker(options, function(err) {
87+
pendingWorker = null;
9688

97-
cb(err);
89+
if (startTimeout) { // timeout no longer needed
90+
clearTimeout(startTimeout);
91+
}
9892

93+
if (!isWorkerTerminated) {
94+
cb(err);
95+
}
9996
});
10097
};
10198
}

lib/commands/upgrade.js

+22-24
Original file line numberDiff line numberDiff line change
@@ -77,52 +77,50 @@ module.exports.more = function(cb) {
7777
function getTask(evt, worker, options) {
7878
return function(cb) {
7979

80-
// kill new worker if takes too long
81-
var newKiller = null;
82-
var newWorker;
83-
var exitListener = function() {
84-
if (newKiller) {
85-
clearTimeout(newKiller);
86-
}
87-
};
88-
var killer;
80+
var pendingWorker;
8981

82+
// kill new worker if takes too long
83+
var newWorkerTimeout = null;
84+
var isNewWorkerTerminated = false;
9085
if (options.timeout > 0) { // start timeout if specified
91-
newKiller = setTimeout(function() {
92-
newWorker.removeListener("exit", exitListener);// remove temp listener
93-
newWorker.kill("SIGKILL"); // go get'em, killer
94-
cb("timed out");
86+
newWorkerTimeout = setTimeout(function() {
87+
if (!pendingWorker) return;
88+
89+
isNewWorkerTerminated = true;
90+
pendingWorker.on('exit', function () {
91+
cb("timed out");
92+
});
93+
pendingWorker.kill("SIGKILL"); // go get'em, killer
9594
}, options.timeout);
9695
}
9796

9897
// lets start new worker
99-
newWorker = evt.service.newWorker(options, function(err, newWorker) {
100-
if (newWorker) { // won't exist if failure
101-
newWorker.removeListener("exit", exitListener); // remove temp listener
102-
}
103-
if (newKiller) { // timeout no longer needed
104-
clearTimeout(newKiller);
98+
pendingWorker = evt.service.newWorker(options, function (err) {
99+
pendingWorker = null;
100+
if (newWorkerTimeout) { // timeout no longer needed
101+
clearTimeout(newWorkerTimeout);
105102
}
103+
106104
if (err) {
107105
cb(err);
108106
return;
109107
}
110108

111109
// ok, lets stop old worker
112-
killer = null;
110+
var oldWorkerTimeout = null;
113111
if (options.timeout > 0) { // start timeout if specified
114-
killer = setTimeout(function() {
112+
oldWorkerTimeout = setTimeout(function() {
115113
worker.kill("SIGKILL"); // go get'em, killer
116114
}, options.timeout);
117115
}
118116

119117
worker.on("exit", function() {
120-
if (killer) {
121-
clearTimeout(killer);
118+
if (oldWorkerTimeout) {
119+
clearTimeout(oldWorkerTimeout);
122120
}
123121

124122
// exit complete, fire callback
125-
setTimeout(cb, 250); // slight delay in case other events are piled up
123+
setImmediate(cb); // slight delay in case other events are piled up
126124
});
127125

128126
require("../workers").exitGracefully(worker);

lib/net-servers.js

+1-14
Original file line numberDiff line numberDiff line change
@@ -108,20 +108,7 @@ function netServersClose(cb) {
108108

109109
function createWaitForCloseTask(server) {
110110
return function(cb) {
111-
var tmpRequestCb = function(req, res) {
112-
if (res.headersSent === false) {
113-
// required to gracefully close connections on pending requests
114-
// this logic is typically only hit under VERY high load
115-
res.setHeader("Connection", "close");
116-
}
117-
};
118-
var tmpCloseCb = function() {
119-
server.removeListener("close", tmpCloseCb);
120-
//server.removeListener("request", tmpRequestCb);
121-
cb(null, true);
122-
};
123-
server.on("close", tmpCloseCb);
124-
//server.on("request", tmpRequestCb);
111+
server.once("close", function() { cb(null, true); });
125112
server.close();
126113
};
127114
}

lib/stop.js

+11-11
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ function stop(timeout, cb) {
1010

1111
if (cservice.workers.length > 0) { // issue shutdown
1212
cservice.trigger("shutdown", function() {
13-
if (cb) cb(null, "Shutting down...");
14-
require("./http-server").close();
15-
if (cservice.options.cli === true) {
16-
process.exit(0);
17-
}
13+
handleWorkersExited(cb);
1814
}, "all", timeout);
1915
} else { // gracefully shutdown
20-
if (cb) cb(null, "Shutting down...");
21-
require("./http-server").close();
22-
cservice.locals.state = 0;
23-
if (cservice.options.cli === true) {
24-
process.exit(0);
25-
}
16+
handleWorkersExited(cb);
17+
}
18+
}
19+
20+
function handleWorkersExited(cb) {
21+
if (cb) cb(null, "Shutting down...");
22+
require("./http-server").close();
23+
cservice.locals.state = 0;
24+
if (cservice.options.cli === true) {
25+
process.exit(1);
2626
}
2727
}

lib/workers.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ function get() {
2323
}
2424
}
2525

26-
workers.send=send;
26+
workers.send = send;
2727

2828
return workers;
2929
}
@@ -103,7 +103,7 @@ function demote() {
103103
*/
104104
function send(){
105105
this.forEach(function(worker){
106-
//worker.send.apply(worker, [].slice.apply(arguments));
106+
worker.send.apply(worker, [].slice.apply(arguments));
107107
});
108108
}
109109

test/start-restart.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,14 @@ if(cservice.isWorker){
8080
});
8181

8282
it('Stop workers', function(done) {
83-
cservice.stop(30000, function() {
83+
cservice.stop(30000, function(err, msg) {
84+
assert.ok(!err, 'Error: ' + err);
8485
assert.equal(
8586
cservice.workers.length,
8687
0,
87-
"0 workers expected, but " + cservice.workers.length + " found"
88+
"0 workers expected, but "
89+
+ cservice.workers.length
90+
+ " found. Message: " + msg
8891
);
8992
done();
9093
});

0 commit comments

Comments
 (0)