-
-
Notifications
You must be signed in to change notification settings - Fork 227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Race condition can result in torrent-stream preventing process exit #198
Comments
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 |
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)
}) |
Trott
added a commit
to Trott/torrent-stream
that referenced
this issue
Sep 22, 2019
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
Should be fixed in 1.2.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If
engine.destroy()
is called before the'ready'
event forengine
, thenrechokeIntervalId
is not set whenclearInterval()
is called, resulting in thesetInterval()
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 intorrent-stream
but if a bug in the user's program, then it's a bug that exists intest/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:
nodejs/node#29504 (comment)
Is this a bug in the
test/tracker.js
, a bug inindex.js
, or a complete misunderstanding on my part?The text was updated successfully, but these errors were encountered: