Skip to content

Commit

Permalink
fix race condition involving engine.destroy() and ready event
Browse files Browse the repository at this point in the history
The 'ready' event sets up an interval timer that is cleared in
`destroy()`. However, in some situations, it is possible to for the
interval to be created after `destroy()` has run, resulting in an
interval timer that is never cleared, and thus an app that does not
exit when it is supposed to. This change fixes the issue and adds a
test for it.

Refs: mafintosh#198
  • Loading branch information
Trott committed Sep 22, 2019
1 parent ade9a4e commit 9a4c7d5
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
4 changes: 3 additions & 1 deletion index.js
Expand Up @@ -554,7 +554,9 @@ var torrentStream = function (link, opts, cb) {
onupdate()
}

rechokeIntervalId = setInterval(onrechoke, RECHOKE_INTERVAL)
if (!destroyed) {
rechokeIntervalId = setInterval(onrechoke, RECHOKE_INTERVAL)
}

process.nextTick(function () {
engine.emit('torrent', torrent)
Expand Down
26 changes: 26 additions & 0 deletions test/tracker.js
Expand Up @@ -108,6 +108,32 @@ test('peer should connect to an alternate tracker', function (t) {
server.listen(54321)
})

// This test seems like it might be better contained in basic.js but I am unable
// to trigger the bug without the addition of the server, so it is much more
// like tests in tracker.js, so that's where I'm putting it. ¯\_(ツ)_/¯
test('calling destroy() before \'ready\' should not hold event loop open', function (t) {
t.plan(5)
var engine = null
var server = new tracker.Server()
server.once('listening', function () {
t.ok(true, 'tracker should be listening')

engine = torrents(torrent, { dht: false, trackers: ['http://127.0.0.1:54321/announce'] })
engine.destroy(function () {
engine.remove(t.ok.bind(t, true, 'should be destroyed'))
})
engine.once('ready', function () {
t.ok(true, 'should be ready')
})
})
server.once('start', function (addr) {
t.equal(addr, '127.0.0.1:6881')

server.close(t.ok.bind(t, true, 'tracker should be closed'))
})
server.listen(54321)
})

test('cleanup', function (t) {
t.plan(2)
fixture.destroy(t.ok.bind(t, true, 'should be destroyed'))
Expand Down

0 comments on commit 9a4c7d5

Please sign in to comment.