Race condition can result in torrent-stream preventing process exit

[Trott]

If engine.destroy() is called before the 'ready' event for engine, then rechokeIntervalId is not set when clearInterval() is called, resulting in the setInterval() running endlessly and the app never exiting. It's not clear to me if this should be considered a bug in the user's program or a bug in torrent-stream but if a bug in the user's program, then it's a bug that exists in test/tracker.js. Run the test multiple times to make it hang, and more recent versions of Node.js seem to be more susceptible to the problem.

Refs: https://github.com/nodejs/node/pull/29504#issuecomment-533552469

Is this a bug in the test/tracker.js, a bug in index.js, or a complete misunderstanding on my part?

[Trott]

I should have mentioned that the problematic test is this one:

test('peer should connect to an alternate tracker', 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.once('ready', function () {
      t.ok(true, 'should be ready')
    })
  })
  server.once('start', function (addr) {
    t.equal(addr, '127.0.0.1:6881')

    engine.destroy(function () {
      engine.remove(t.ok.bind(t, true, 'should be destroyed'))
    })
    server.close(t.ok.bind(t, true, 'tracker should be closed'))
  })
  server.listen(54321)
})

All the other ones have the detroy() call inside the ready() listener, but this one does not.

[Trott]

Here's a modification that seems to reliably demonstrate the problem. The test will pass but won't exit.

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)
})

[mafintosh]

Should be fixed in 1.2.0