Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix multiple instances bug #29

Merged
merged 1 commit into from

2 participants

Sebastian Gierlinger Eric Clemmons
Sebastian Gierlinger

closes #28

  • added stopdone to wait for server to stop
  • renamed done to startdone
  • added testsequence which is able to reproduce the bug without my changes to server.js
Sebastian Gierlinger sebgie Fix multiple instances bug
closes #28
- added stopdone to wait for server to stop
- renamed done to startdone
- added testsequence which is able to reproduce the bug without my
changes
58743e3
Eric Clemmons ericclemmons merged commit 06fb97f into from
Eric Clemmons
Owner

Thanks for this! I didn't even think about this issue!

Eric Clemmons
Owner

v0.4.8 is published with your fix! Thank you!

Sebastian Gierlinger

np, thank you for merging/publishing almost instantly :+1:

Sebastian Gierlinger sebgie deleted the branch
Eric Clemmons
Owner

It was the tests :) When there's a PR without tests, it takes me a while to understand the actual problem, but you described that really freakin' well, too!

Eric Clemmons
Owner

Ok, there's an issue reported in #30, but your changes to Gruntfile.js doesn't exhibit a problem when using v0.4.7. I may need to revert unless you can help resolve the issue in #30, or help illustrate the problem you were having (that I can't replicate currently) so both issues are resolved.

Eric Clemmons ericclemmons referenced this pull request from a commit
Eric Clemmons Revert "Merge pull request #29 from sebgie/issue#28"
This reverts commit 06fb97f, reversing
changes made to 1714ca8.
1a5fbaa
Sebastian Gierlinger

Sorry for causing problems :-/. I'll have a look at it in the morning.

Eric Clemmons
Owner

You little trouble-maker you! :)

Sebastian Gierlinger

Ok, I see the following error when adding the test sequence in gruntfile.js. I'm testing with:

  • OS X 10.9
  • node v0.10.22
  • grunt-cli v0.1.9
  • grunt v0.4.2
Running "nodeunit:custom_delay" (nodeunit) task
Testing custom_delay_test.js..OK
>> 4 assertions passed (44ms)

Running "express:custom_delay:stop" (express) task
Stopping Express server

Running "express:custom_delay" (express) task
Starting background Express server

Running "nodeunit:custom_delay" (nodeunit) task
Testing custom_delay_test.jsFF
>> custom_delay - test_runs_after_timeout
>> Error: Expected 2 assertions, 0 ran
>> at ClientRequest.<anonymous> (test/custom_delay_test.js:22:12)
>> at ClientRequest.EventEmitter.emit (events.js:95:17)
>> at Socket.socketErrorListener (http.js:1520:9)
>> at Socket.EventEmitter.emit (events.js:95:17)
>> at net.js:426:14
>> at process._tickCallback (node.js:415:13)

>> custom_delay - test_runs_in_development
>> Error: Expected 2 assertions, 0 ran
>> at ClientRequest.<anonymous> (test/custom_delay_test.js:34:12)
>> at ClientRequest.EventEmitter.emit (events.js:95:17)
>> at Socket.socketErrorListener (http.js:1520:9)
>> at Socket.EventEmitter.emit (events.js:95:17)
>> at net.js:426:14
>> at process._tickCallback (node.js:415:13)

Warning: 2/2 assertions failed (13ms) Use --force to continue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 5, 2013
  1. Sebastian Gierlinger

    Fix multiple instances bug

    sebgie authored
    closes #28
    - added stopdone to wait for server to stop
    - renamed done to startdone
    - added testsequence which is able to reproduce the bug without my
    changes
This page is out of date. Refresh to see the latest.
Showing with 20 additions and 8 deletions.
  1. +5 −0 Gruntfile.js
  2. +15 −8 tasks/lib/server.js
5 Gruntfile.js
View
@@ -155,6 +155,11 @@ module.exports = function(grunt) {
'express:custom_output', 'nodeunit:custom_output', 'express:custom_output:stop',
'express:stoppable', 'express:stoppable:stop', 'nodeunit:stoppable',
+ // Multiple consecutive starts/stops of the same server
+ 'express:custom_delay', 'nodeunit:custom_delay', 'express:custom_delay:stop',
+ 'express:custom_delay', 'nodeunit:custom_delay', 'express:custom_delay:stop',
+ 'express:custom_delay', 'nodeunit:custom_delay', 'express:custom_delay:stop',
+
// Multiple servers
'express:custom_port', 'express:defaults',
'nodeunit:defaults', 'nodeunit:custom_port',
23 tasks/lib/server.js
View
@@ -13,15 +13,16 @@ module.exports = function(grunt, target) {
process._servers = {};
}
- var backup = null;
- var done = null;
- var server = process._servers[target]; // Store server between live reloads to close/restart express
+ var backup = null;
+ var startdone = null;
+ var stopdone = null;
+ var server = process._servers[target]; // Store server between live reloads to close/restart express
var finished = function() {
- if (done) {
- done();
+ if (startdone) {
+ startdone();
- done = null;
+ startdone = null;
}
};
@@ -50,7 +51,7 @@ module.exports = function(grunt, target) {
grunt.log.writeln('Starting '.cyan + (options.background ? 'background' : 'foreground') + ' Express server');
- done = grunt.task.current.async();
+ startdone = grunt.task.current.async();
// Set PORT for new processes
process.env.PORT = options.port;
@@ -71,7 +72,12 @@ module.exports = function(grunt, target) {
args: options.args,
env: process.env,
fallback: options.fallback
- }, finished);
+ }, function (error, result, code) {
+ if (stopdone) {
+ stopdone();
+ }
+ finished();
+ });
if (options.delay) {
setTimeout(finished, options.delay);
@@ -102,6 +108,7 @@ module.exports = function(grunt, target) {
if (server && server.kill) {
grunt.log.writeln('Stopping'.red + ' Express server');
+ stopdone = grunt.task.current.async();
server.kill('SIGTERM');
process.removeAllListeners();
server = process._servers[target] = null;
Something went wrong with that request. Please try again.