From 9a4c7d5f697f2d2cf27f714adc108182b804bdda Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 21 Sep 2019 23:11:33 -0700 Subject: [PATCH] fix race condition involving engine.destroy() and `ready` event 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: https://github.com/mafintosh/torrent-stream/issues/198 --- index.js | 4 +++- test/tracker.js | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 0080afa..40c0abf 100644 --- a/index.js +++ b/index.js @@ -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) diff --git a/test/tracker.js b/test/tracker.js index 2dc6bec..5156a8b 100644 --- a/test/tracker.js +++ b/test/tracker.js @@ -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'))