Skip to content

Commit

Permalink
Remove workers from cache when they exit
Browse files Browse the repository at this point in the history
Ensures crashed workers are not used for new lint invocations.
  • Loading branch information
novemberborn committed Nov 5, 2016
1 parent 83a3683 commit 415ac43
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 2 deletions.
18 changes: 16 additions & 2 deletions lib/getLinter.js
Expand Up @@ -47,8 +47,15 @@ var createLinter = function (linterName, projectRoot) {
})
},
shutdown: function () {
child.disconnect()
}
// Shutdown is optimistic, but may be called when the linter is
// intentionally removed from the cache. Ignore any errors thrown.
try {
child.disconnect()
} catch (err) {}
},
exited: new Promise(resolve => {
child.once('exit', resolve)
})
}
}

Expand All @@ -60,6 +67,13 @@ module.exports = function getLinter (linterName, projectRoot) {
if (!linter) {
linter = createLinter(linterName, projectRoot)
linters.set(cacheKey, linter)
linter.exited.then(() => {
// A new linter may have been created in the meantime, make sure not to
// delete that one.
if (linters.peek(cacheKey) === linter) {
linters.del(cacheKey)
}
})
}

resolve(linter)
Expand Down
39 changes: 39 additions & 0 deletions test/lib/getLinter.spec.js
Expand Up @@ -81,9 +81,48 @@ describe('lib/getLinter', () => {
getLinter('linter', '/')
forks.linter.emit('message', { id: 10 })
})
it('should clean up linters that exit', () => {
return getLinter('first', '/').then(first => {
return expect(getLinter('first', '/'), 'when fulfilled', 'to be', first)
.then(() => {
forks.first.emit('exit')
return new Promise(resolve => setTimeout(resolve, 10))
})
.then(() => {
const old = first
return getLinter('first', '/').then(first => {
expect(first, 'not to be', old)

// Ensure first is purged from the cache
getLinter('second', '/')
getLinter('third', '/')

const child = forks.first
expect(child, 'to have property', 'wasDisconnected', true)
const promise = getLinter('first', '/')

child.emit('exit')
return new Promise(resolve => setTimeout(resolve, 10))
.then(() => promise)
.then(first => {
return expect(getLinter('first', '/'), 'when fulfilled', 'to be', first)
})
})
})
})
})
it('should ignore disconnect errors', () => {
return getLinter('first', '/').then(linter => {
forks.first.disconnect = () => { throw new Error('ignore me') }
expect(() => linter.shutdown(), 'not to throw')
})
})

describe('cleanLinters()', () => {
it('should shut down all workers', () => {
// Reset state
getLinter.cleanLinters()

getLinter('first', '/')
getLinter('second', '/')
expect(forks, 'to satisfy', {
Expand Down

0 comments on commit 415ac43

Please sign in to comment.