From c0fa49aa7c56f14a3836986e8629411a72515a78 Mon Sep 17 00:00:00 2001 From: Lukasz Zatorski Date: Fri, 15 Nov 2013 13:49:58 -0800 Subject: [PATCH] feat(launcher): send SIGKILL if SIGINT does not kill the browser In some cases chrome on linux does not die when it receives a SIGINT signal. So after a SIGINT is sent, wait 2 more seconds and issue a SIGKILL if the process is still running. --- lib/launchers/Base.js | 34 +++++++++++++++++++++------- test/unit/launchers/Base.spec.coffee | 3 +-- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/launchers/Base.js b/lib/launchers/Base.js index 581021822..cb5891450 100644 --- a/lib/launchers/Base.js +++ b/lib/launchers/Base.js @@ -16,14 +16,14 @@ var BEING_TIMEOUTED = 5; var BaseBrowser = function(id, emitter, captureTimeout, retryLimit) { var self = this; var capturingUrl; - var exitCallback = function() {}; + var exitCallbacks = []; + this.killTimeout = 2000; this.id = id; this.state = null; this._tempDir = path.normalize((env.TMPDIR || env.TMP || env.TEMP || '/tmp') + '/karma-' + id.toString()); - this.start = function(url) { capturingUrl = url; self.state = BEING_CAPTURED; @@ -59,19 +59,32 @@ var BaseBrowser = function(id, emitter, captureTimeout, retryLimit) { this.kill = function(callback) { - exitCallback = callback || function() {}; + var exitCallback = callback || function() {}; log.debug('Killing %s', self.name); - - if (self.state !== FINISHED) { + if (self.state === FINISHED) { + process.nextTick(exitCallback); + } else if (self.state === BEING_KILLED) { + exitCallbacks.push(exitCallback); + } else { self.state = BEING_KILLED; self._process.kill(); - } else { - process.nextTick(exitCallback); + exitCallbacks.push(exitCallback); + setTimeout(self._onKillTimeout, self.killTimeout); } }; + this._onKillTimeout = function() { + if (self.state !== BEING_KILLED) { + return; + } + + log.warn('%s was not killed in %d ms, sending SIGKILL.', self.name, self.killTimeout); + + self._process.kill('SIGKILL'); + }; + this._onTimeout = function() { if (self.state !== BEING_CAPTURED) { return; @@ -169,7 +182,12 @@ var BaseBrowser = function(id, emitter, captureTimeout, retryLimit) { } self.state = FINISHED; - self._cleanUpTmp(exitCallback); + self._cleanUpTmp(function(err) { + exitCallbacks.forEach(function(exitCallback) { + exitCallback(err); + }); + exitCallbacks = []; + }); }; diff --git a/test/unit/launchers/Base.spec.coffee b/test/unit/launchers/Base.spec.coffee index c33f5344b..8a59fa4ba 100644 --- a/test/unit/launchers/Base.spec.coffee +++ b/test/unit/launchers/Base.spec.coffee @@ -156,8 +156,7 @@ describe 'launchers Base', -> expect(killSpy).not.to.have.been.called mockSpawn._processes[0].emit 'close', 0 - expect(mockRimraf).to.have.been.calledWith path.normalize('/tmp/karma-12345'), killSpy - + expect(mockRimraf).to.have.been.calledWith path.normalize('/tmp/karma-12345') mockRimraf._callbacks[0]() # rm tempdir expect(killSpy).to.have.been.called