From 7c1189cf3c79618dc9f1ae1e22841362f3af30e7 Mon Sep 17 00:00:00 2001 From: jason allen Date: Thu, 15 Dec 2016 09:27:27 -0800 Subject: [PATCH 1/4] [FIX] worker destroy will really stop it. --- src/worker.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/worker.js b/src/worker.js index da5440e..b26c162 100644 --- a/src/worker.js +++ b/src/worker.js @@ -31,6 +31,7 @@ export default class Job extends events.EventEmitter { this.checkInterval = opts.interval || 1500; this.checkSize = opts.size || 10; this.doctype = opts.doctype || constants.DEFAULT_SETTING_DOCTYPE; + this.running = true; this.debug = (...msg) => debug(...msg, `id: ${this.id}`); @@ -40,6 +41,7 @@ export default class Job extends events.EventEmitter { } destroy() { + this.running = false; clearInterval(this._checker); } @@ -237,6 +239,9 @@ export default class Job extends events.EventEmitter { } _startJobPolling() { + if (!this.running) { + return; + } this._checker = setInterval(() => { this._getPendingJobs() .then((jobs) => this._claimPendingJobs(jobs)); @@ -335,4 +340,4 @@ export default class Job extends events.EventEmitter { this.emit(constants.EVENT_WORKER_JOB_SEARCH_ERROR, this._formatErrorParams(err)); }); } -} \ No newline at end of file +} From 4befaee4ccb9f4f60aa09e7d5dc907d864e9afe7 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 16 Dec 2016 16:42:18 -0700 Subject: [PATCH 2/4] minor syntax change make running more clearly a private state variable --- src/worker.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/worker.js b/src/worker.js index b26c162..94f34e3 100644 --- a/src/worker.js +++ b/src/worker.js @@ -31,17 +31,17 @@ export default class Job extends events.EventEmitter { this.checkInterval = opts.interval || 1500; this.checkSize = opts.size || 10; this.doctype = opts.doctype || constants.DEFAULT_SETTING_DOCTYPE; - this.running = true; this.debug = (...msg) => debug(...msg, `id: ${this.id}`); this._checker = false; + this._running = true; this.debug(`Created worker for job type ${this.jobtype}`); this._startJobPolling(); } destroy() { - this.running = false; + this._running = false; clearInterval(this._checker); } @@ -239,9 +239,10 @@ export default class Job extends events.EventEmitter { } _startJobPolling() { - if (!this.running) { + if (!this._running) { return; } + this._checker = setInterval(() => { this._getPendingJobs() .then((jobs) => this._claimPendingJobs(jobs)); From c64c5c80d6218b33ec6e42de025e389878309d28 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 16 Dec 2016 16:50:48 -0700 Subject: [PATCH 3/4] call _stopJobPolling on destroy --- src/worker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/worker.js b/src/worker.js index 94f34e3..00bf410 100644 --- a/src/worker.js +++ b/src/worker.js @@ -42,7 +42,7 @@ export default class Job extends events.EventEmitter { destroy() { this._running = false; - clearInterval(this._checker); + this._stopJobPolling(); } toJSON() { From b2f4ae857f6bcec132252a75c88605f00fe445a8 Mon Sep 17 00:00:00 2001 From: Joe Fleming Date: Fri, 16 Dec 2016 16:53:22 -0700 Subject: [PATCH 4/4] test that the running state prevents searches --- test/src/worker.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/src/worker.js b/test/src/worker.js index 0c4df54..bfb7b02 100644 --- a/test/src/worker.js +++ b/test/src/worker.js @@ -185,6 +185,27 @@ describe('Worker class', function () { clock.tick(interval); sinon.assert.calledOnce(searchSpy); }); + + it('should not poll once destroyed', function () { + const worker = new Worker(mockQueue, 'test', noop); + + // move the clock a couple times, test for searches each time + sinon.assert.notCalled(searchSpy); + clock.tick(defaults.interval); + sinon.assert.calledOnce(searchSpy); + clock.tick(defaults.interval); + sinon.assert.calledTwice(searchSpy); + + // destroy the worker, move the clock, make sure another search doesn't happen + worker.destroy(); + clock.tick(defaults.interval); + sinon.assert.calledTwice(searchSpy); + + // manually call job poller, move the clock, make sure another search doesn't happen + worker._startJobPolling(); + clock.tick(defaults.interval); + sinon.assert.calledTwice(searchSpy); + }); }); describe('query for pending jobs', function () {