From 27ae1481678fadadc257f11bc7c2125a5a603962 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:23:46 -0800 Subject: [PATCH 01/15] Fixes for PR #185 --- lib/server/parse-http.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server/parse-http.js b/lib/server/parse-http.js index 24c3c69..bf6cb2c 100644 --- a/lib/server/parse-http.js +++ b/lib/server/parse-http.js @@ -25,7 +25,7 @@ function parseHttpRequest (req, opts) { if (!params.port) throw new Error('invalid port') params.left = Number(params.left) - if (isNaN(params.left)) params.left = Infinity + if (Number.isNaN(params.left)) params.left = Infinity params.compact = Number(params.compact) || 0 params.numwant = Math.min( From c2c8e36af7299454e9f5cff2deb73a06875e0d16 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:24:40 -0800 Subject: [PATCH 02/15] Fix bug where left = 0 would be converted to Infinity Possibly fixes: https://github.com/feross/bittorrent-tracker/issues/196 --- lib/server/parse-websocket.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/server/parse-websocket.js b/lib/server/parse-websocket.js index 0db50b5..b6fecb3 100644 --- a/lib/server/parse-websocket.js +++ b/lib/server/parse-websocket.js @@ -28,7 +28,9 @@ function parseWebSocketRequest (socket, opts, params) { params.to_peer_id = common.binaryToHex(params.to_peer_id) } - params.left = Number(params.left) || Infinity + params.left = Number(params.left) + if (Number.isNaN(params.left)) params.left = Infinity + params.numwant = Math.min( Number(params.offers && params.offers.length) || 0, // no default - explicit only common.MAX_ANNOUNCE_PEERS From ed3da2f39b581f9972b73f0ba9e0b43273f68276 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:25:46 -0800 Subject: [PATCH 03/15] If peer is already complete, it should still be refreshed in the LRU cache --- lib/server/swarm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index f8d8a44..6480052 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -102,8 +102,8 @@ Swarm.prototype._onAnnounceUpdate = function (params, peer, id) { this.complete += 1 this.incomplete -= 1 peer.complete = true - this.peers.set(id, peer) } + this.peers.set(id, peer) } Swarm.prototype._getPeers = function (numwant, ownPeerId, isWebRTC) { From 62dd0f7df22d99454dd9816ed7644a5a4af3f32c Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:26:03 -0800 Subject: [PATCH 04/15] treat unexpected 'completed' events as 'updated' --- lib/server/swarm.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index 6480052..d8a26e5 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -82,8 +82,8 @@ Swarm.prototype._onAnnounceCompleted = function (params, peer, id) { return this._onAnnounceStarted(params, peer, id) // treat as a start } if (peer.complete) { - debug('unexpected `completed` event from peer that is already marked as completed') - return // do nothing + debug('unexpected `completed` event from peer that is already completed') + return this._onAnnounceUpdate(params, peer, id) // treat as an update } this.complete += 1 From 6351f2b260b7579c4a98aec5610fbbd43370dde1 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:40:36 -0800 Subject: [PATCH 05/15] style: re-order methods --- server.js | 106 +++++++++++++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/server.js b/server.js index 6ba51a7..bf6c12b 100644 --- a/server.js +++ b/server.js @@ -575,6 +575,59 @@ Server.prototype._onWebSocketRequest = function (socket, opts, params) { }) } +Server.prototype._onWebSocketSend = function (socket, err) { + var self = this + if (err) self._onWebSocketError(socket, err) +} + +Server.prototype._onWebSocketClose = function (socket) { + var self = this + debug('websocket close %s', socket.peerId) + + if (socket.peerId) { + socket.infoHashes.forEach(function (infoHash) { + var swarm = self.torrents[infoHash] + if (swarm) { + swarm.announce({ + type: 'ws', + event: 'stopped', + numwant: 0, + peer_id: socket.peerId + }, noop) + } + }) + } + + // ignore all future errors + socket.onSend = noop + socket.on('error', noop) + + socket.peerId = null + socket.infoHashes = null + + if (typeof socket.onMessageBound === 'function') { + socket.removeListener('message', socket.onMessageBound) + } + socket.onMessageBound = null + + if (typeof socket.onErrorBound === 'function') { + socket.removeListener('error', socket.onErrorBound) + } + socket.onErrorBound = null + + if (typeof socket.onCloseBound === 'function') { + socket.removeListener('close', socket.onCloseBound) + } + socket.onCloseBound = null +} + +Server.prototype._onWebSocketError = function (socket, err) { + var self = this + debug('websocket error %s', err.message || err) + self.emit('warning', err) + self._onWebSocketClose(socket) +} + Server.prototype._onRequest = function (params, cb) { var self = this if (params && params.action === common.ACTIONS.CONNECT) { @@ -757,59 +810,6 @@ function makeUdpPacket (params) { return packet } -Server.prototype._onWebSocketSend = function (socket, err) { - var self = this - if (err) self._onWebSocketError(socket, err) -} - -Server.prototype._onWebSocketClose = function (socket) { - var self = this - debug('websocket close %s', socket.peerId) - - if (socket.peerId) { - socket.infoHashes.forEach(function (infoHash) { - var swarm = self.torrents[infoHash] - if (swarm) { - swarm.announce({ - type: 'ws', - event: 'stopped', - numwant: 0, - peer_id: socket.peerId - }, noop) - } - }) - } - - // ignore all future errors - socket.onSend = noop - socket.on('error', noop) - - socket.peerId = null - socket.infoHashes = null - - if (typeof socket.onMessageBound === 'function') { - socket.removeListener('message', socket.onMessageBound) - } - socket.onMessageBound = null - - if (typeof socket.onErrorBound === 'function') { - socket.removeListener('error', socket.onErrorBound) - } - socket.onErrorBound = null - - if (typeof socket.onCloseBound === 'function') { - socket.removeListener('close', socket.onCloseBound) - } - socket.onCloseBound = null -} - -Server.prototype._onWebSocketError = function (socket, err) { - var self = this - debug('websocket error %s', err.message || err) - self.emit('warning', err) - self._onWebSocketClose(socket) -} - function toNumber (x) { x = Number(x) return x >= 0 ? x : false From 8f33b95f9f0f961e9e2db7e961a348c615e91abb Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:40:52 -0800 Subject: [PATCH 06/15] swarm maxAge: increase to 20 min --- lib/server/swarm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index d8a26e5..738005f 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -9,7 +9,7 @@ var randomIterate = require('random-iterate') function Swarm (infoHash, server) { this.peers = new LRU({ max: server.peersCacheLength || 1000, - maxAge: server.peersCacheTtl || 900000 // 900 000ms = 15 minutes + maxAge: server.peersCacheTtl || 20 * 60 * 1000 // 20 minutes }) this.complete = 0 this.incomplete = 0 From 70750888483bd184275f3a1c17a2c38fa5b03047 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 17:43:16 -0800 Subject: [PATCH 07/15] Close websockets when peers are evicted from LRU cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Possibly fixes: https://github.com/feross/bittorrent-tracker/issues/196 Close websockets when peers are evicted from LRU cache, otherwise it's possible for a peer object to be evicted from the LRU cache without the socket being cleaned up. That will leak memory until the websocket is closed by the remote client. It also messes up the stats. --- lib/server/swarm.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index 738005f..0b39918 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -7,12 +7,24 @@ var randomIterate = require('random-iterate') // Regard this as the default implementation of an interface that you // need to support when overriding Server.createSwarm() and Server.getSwarm() function Swarm (infoHash, server) { + this.complete = 0 + this.incomplete = 0 this.peers = new LRU({ max: server.peersCacheLength || 1000, maxAge: server.peersCacheTtl || 20 * 60 * 1000 // 20 minutes }) - this.complete = 0 - this.incomplete = 0 + + // When a peer is evicted from the LRU cache, if it's a websocket peer, + // close the websocket. + this.peers.on('evict', function (data) { + var peer = data.value + if (peer.socket) { + try { + peer.socket.close() + peer.socket = null + } catch (err) {} + } + }) } Swarm.prototype.announce = function (params, cb) { From 1b22b53fe84c9b1ea1925de82f716ceb588949b6 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 18:20:26 -0800 Subject: [PATCH 08/15] fix tests: wait for socket to send final responses --- lib/server/swarm.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index 0b39918..d2e1c5f 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -14,15 +14,18 @@ function Swarm (infoHash, server) { maxAge: server.peersCacheTtl || 20 * 60 * 1000 // 20 minutes }) - // When a peer is evicted from the LRU cache, if it's a websocket peer, - // close the websocket. + // When a websocket peer is evicted from the LRU cache, close the websocket + // after a short timeout period. We wait 1s so the server has a chance to send + // a response to 'stopped' events, which remove the peer and cause an eviction. this.peers.on('evict', function (data) { var peer = data.value if (peer.socket) { - try { - peer.socket.close() - peer.socket = null - } catch (err) {} + setTimeout(function () { + try { + peer.socket.close() + peer.socket = null + } catch (err) {} + }, 1000) } }) } From 29d4564bbde01a01a40ec2ba7cf008e94f726049 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 20:28:38 -0800 Subject: [PATCH 09/15] remove unneeded timeout --- lib/server/swarm.js | 10 ++++------ test/server.js | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index d2e1c5f..c8d52cc 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -20,12 +20,10 @@ function Swarm (infoHash, server) { this.peers.on('evict', function (data) { var peer = data.value if (peer.socket) { - setTimeout(function () { - try { - peer.socket.close() - peer.socket = null - } catch (err) {} - }, 1000) + try { + peer.socket.close() + peer.socket = null + } catch (err) {} } }) } diff --git a/test/server.js b/test/server.js index cc679eb..099b403 100644 --- a/test/server.js +++ b/test/server.js @@ -25,8 +25,7 @@ function serverTest (t, serverType, serverFamily) { : '127.0.0.1' var opts = { - serverType: serverType, - peersCacheLength: 2 + serverType: serverType } common.createServer(t, opts, function (server) { From d534582a8cc12616e30062d279520b301a6d51f8 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Thu, 2 Feb 2017 20:51:07 -0800 Subject: [PATCH 10/15] Only close websocket when it's not participating in any more swarms --- lib/server/swarm.js | 28 ++++++++++++++++++++++++---- server.js | 2 +- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index c8d52cc..6f08543 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -7,19 +7,29 @@ var randomIterate = require('random-iterate') // Regard this as the default implementation of an interface that you // need to support when overriding Server.createSwarm() and Server.getSwarm() function Swarm (infoHash, server) { + this.infoHash = infoHash this.complete = 0 this.incomplete = 0 + this.peers = new LRU({ max: server.peersCacheLength || 1000, maxAge: server.peersCacheTtl || 20 * 60 * 1000 // 20 minutes }) - // When a websocket peer is evicted from the LRU cache, close the websocket - // after a short timeout period. We wait 1s so the server has a chance to send - // a response to 'stopped' events, which remove the peer and cause an eviction. + // When a peer is evicted from the LRU store, send a synthetic 'stopped' event + // so the stats get updated correctly. this.peers.on('evict', function (data) { var peer = data.value - if (peer.socket) { + this.announce({ + type: peer.type, + event: 'stopped', + numwant: 0, + peer_id: peer.peerId + }, noop) + + // When a websocket peer is evicted, and it's not in any other swarms, close + // the websocket to conserve server resources. + if (peer.socket && peer.socket.infoHashes.length === 0) { try { peer.socket.close() peer.socket = null @@ -86,6 +96,14 @@ Swarm.prototype._onAnnounceStopped = function (params, peer, id) { if (peer.complete) this.complete -= 1 else this.incomplete -= 1 + + // If it's a websocket, remove this swarm's infohash from the list of active + // swarms that this peer is participating in. + if (peer.socket) { + var index = peer.socket.infoHashes.indexOf(this.infoHash) + peer.socket.infoHashes.splice(index, 1) + } + this.peers.remove(id) } @@ -133,3 +151,5 @@ Swarm.prototype._getPeers = function (numwant, ownPeerId, isWebRTC) { } return peers } + +function noop () {} diff --git a/server.js b/server.js index bf6c12b..d28b692 100644 --- a/server.js +++ b/server.js @@ -585,7 +585,7 @@ Server.prototype._onWebSocketClose = function (socket) { debug('websocket close %s', socket.peerId) if (socket.peerId) { - socket.infoHashes.forEach(function (infoHash) { + socket.infoHashes.slice(0).forEach(function (infoHash) { var swarm = self.torrents[infoHash] if (swarm) { swarm.announce({ From e7311062793fea1958da4fc9c579777a24a14982 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 8 Feb 2017 13:11:35 -0800 Subject: [PATCH 11/15] test: ensure electron-webrtc is started only for test where it's needed before this change, it was getting initialized immediately, since it was outside a tape test block --- test/server.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/test/server.js b/test/server.js index 099b403..3470537 100644 --- a/test/server.js +++ b/test/server.js @@ -2,12 +2,9 @@ var Buffer = require('safe-buffer').Buffer var Client = require('../') var common = require('./common') var test = require('tape') -var wrtc = require('electron-webrtc')() +var electronWebrtc = require('electron-webrtc') -var wrtcReady = false -wrtc.electronDaemon.once('ready', function () { - wrtcReady = true -}) +var wrtc var infoHash = '4cb67059ed6bd08362da625b3ae77f6f4a075705' var peerId = Buffer.from('01234567890123456789') @@ -177,15 +174,11 @@ test('udp server', function (t) { }) test('ws server', function (t) { - if (wrtcReady) { - runTest() - } else { - wrtc.electronDaemon.once('ready', runTest) - } - function runTest () { - t.once('end', function () { - wrtc.close() - }) + wrtc = electronWebrtc() + wrtc.electronDaemon.once('ready', function () { serverTest(t, 'ws', 'inet') - } + }) + t.once('end', function () { + wrtc.close() + }) }) From 4422067607b9fc9f7094d96b58de20d93a73b7cb Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 8 Feb 2017 13:11:48 -0800 Subject: [PATCH 12/15] test: server: check that all clients, server are destroyed --- test/server.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/server.js b/test/server.js index 3470537..73e9fe6 100644 --- a/test/server.js +++ b/test/server.js @@ -12,7 +12,7 @@ var peerId2 = Buffer.from('12345678901234567890') var peerId3 = Buffer.from('23456789012345678901') function serverTest (t, serverType, serverFamily) { - t.plan(36) + t.plan(40) var hostname = serverFamily === 'inet6' ? '[::1]' @@ -138,16 +138,23 @@ function serverTest (t, serverType, serverFamily) { t.equal(data.incomplete, 1) client2.destroy(function () { + t.pass('client2 destroyed') client3.stop() client3.once('update', function (data) { t.equal(data.announce, announceUrl) t.equal(data.complete, 1) t.equal(data.incomplete, 0) + client1.destroy(function () { + t.pass('client1 destroyed') + }) + client3.destroy(function () { - client1.destroy(function () { - server.close() - }) + t.pass('client3 destroyed') + }) + + server.close(function () { + t.pass('server destroyed') }) }) }) From cd4a976121d89cabaa8fdfcfad6f67822cc6e83b Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 8 Feb 2017 13:12:39 -0800 Subject: [PATCH 13/15] add failing tests for desired evicition behavior --- test/evict.js | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 test/evict.js diff --git a/test/evict.js b/test/evict.js new file mode 100644 index 0000000..1ff60bb --- /dev/null +++ b/test/evict.js @@ -0,0 +1,126 @@ +var Buffer = require('safe-buffer').Buffer +var Client = require('../') +var common = require('./common') +var test = require('tape') +var electronWebrtc = require('electron-webrtc') + +var wrtc + +var infoHash = '4cb67059ed6bd08362da625b3ae77f6f4a075705' +var peerId = Buffer.from('01234567890123456789') +var peerId2 = Buffer.from('12345678901234567890') +var peerId3 = Buffer.from('23456789012345678901') + +function serverTest (t, serverType, serverFamily) { + t.plan(10) + + var hostname = serverFamily === 'inet6' + ? '[::1]' + : '127.0.0.1' + + var opts = { + serverType: serverType, + peersCacheLength: 2 // LRU cache can only contain a max of 2 peers + } + + common.createServer(t, opts, function (server) { + // Not using announceUrl param from `common.createServer()` since we + // want to control IPv4 vs IPv6. + var port = server[serverType].address().port + var announceUrl = serverType + '://' + hostname + ':' + port + '/announce' + + var client1 = new Client({ + infoHash: infoHash, + announce: [ announceUrl ], + peerId: peerId, + port: 6881, + wrtc: wrtc + }) + if (serverType === 'ws') common.mockWebsocketTracker(client1) + + client1.start() + + client1.once('update', function (data) { + var client2 = new Client({ + infoHash: infoHash, + announce: [ announceUrl ], + peerId: peerId2, + port: 6882, + wrtc: wrtc + }) + if (serverType === 'ws') common.mockWebsocketTracker(client2) + + client2.start() + + client2.once('update', function (data) { + server.getSwarm(infoHash, function (err, swarm) { + t.error(err) + + t.equal(swarm.complete + swarm.incomplete, 2) + + // Ensure that first peer is evicted when a third one is added + var evicted = false + swarm.peers.once('evict', function (evictedPeer) { + t.equal(evictedPeer.value.peerId, peerId.toString('hex')) + t.equal(swarm.complete + swarm.incomplete, 2) + evicted = true + }) + + var client3 = new Client({ + infoHash: infoHash, + announce: [ announceUrl ], + peerId: peerId3, + port: 6880, + wrtc: wrtc + }) + if (serverType === 'ws') common.mockWebsocketTracker(client3) + + client3.start() + + client3.once('update', function (data) { + t.ok(evicted, 'client1 was evicted from server before client3 gets response') + t.equal(swarm.complete + swarm.incomplete, 2) + + client1.destroy(function () { + t.pass('client1 destroyed') + }) + + client2.destroy(function () { + t.pass('client3 destroyed') + }) + + client3.destroy(function () { + t.pass('client3 destroyed') + }) + + server.close(function () { + t.pass('server destroyed') + }) + }) + }) + }) + }) + }) +} + +test('evict: ipv4 server', function (t) { + serverTest(t, 'http', 'inet') +}) + +test('evict: http ipv6 server', function (t) { + serverTest(t, 'http', 'inet6') +}) + +test('evict: udp server', function (t) { + serverTest(t, 'udp', 'inet') +}) + +test('evict: ws server', function (t) { + wrtc = electronWebrtc() + wrtc.electronDaemon.once('ready', function () { + serverTest(t, 'ws', 'inet') + }) + t.once('end', function () { + wrtc.close() + }) +}) From 806ce1d18ba736fdd5cae581ca1d840f21801299 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 8 Feb 2017 13:13:20 -0800 Subject: [PATCH 14/15] Address @DiegoRBaquero's feedback From comment: https://github.com/feross/bittorrent-tracker/pull/198#discussion_r993882 95 --- lib/server/swarm.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/server/swarm.js b/lib/server/swarm.js index 6f08543..5203318 100644 --- a/lib/server/swarm.js +++ b/lib/server/swarm.js @@ -7,25 +7,27 @@ var randomIterate = require('random-iterate') // Regard this as the default implementation of an interface that you // need to support when overriding Server.createSwarm() and Server.getSwarm() function Swarm (infoHash, server) { - this.infoHash = infoHash - this.complete = 0 - this.incomplete = 0 + var self = this + self.infoHash = infoHash + self.complete = 0 + self.incomplete = 0 - this.peers = new LRU({ + self.peers = new LRU({ max: server.peersCacheLength || 1000, maxAge: server.peersCacheTtl || 20 * 60 * 1000 // 20 minutes }) // When a peer is evicted from the LRU store, send a synthetic 'stopped' event // so the stats get updated correctly. - this.peers.on('evict', function (data) { + self.peers.on('evict', function (data) { var peer = data.value - this.announce({ + var params = { type: peer.type, event: 'stopped', numwant: 0, peer_id: peer.peerId - }, noop) + } + self._onAnnounceStopped(params, peer, peer.peerId) // When a websocket peer is evicted, and it's not in any other swarms, close // the websocket to conserve server resources. @@ -151,5 +153,3 @@ Swarm.prototype._getPeers = function (numwant, ownPeerId, isWebRTC) { } return peers } - -function noop () {} From 3f3db7deb1f7b1e1a6364990ba4fcfc2d4bd6130 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 8 Feb 2017 13:20:41 -0800 Subject: [PATCH 15/15] client: socketPool should not be shared across clients Caught this issue because of the new eviction tests. Essentially, this change moves the socketPool into the client instance instead of a reused variable at the module level. When a client sends stop (or is evicted) the server will close the websocket connection if that client is not in any other swarms (based on peerId). However, if we are using a single socket for multiple clients (as was the case before this commit), then other clients will have their sockets unintentionally closed by the server. --- client.js | 5 +++++ lib/client/websocket-tracker.js | 21 ++++++++------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/client.js b/client.js index cea05be..f29e962 100644 --- a/client.js +++ b/client.js @@ -69,6 +69,11 @@ function Client (opts) { // See: https://github.com/feross/webtorrent-hybrid/issues/46 self._wrtc = typeof opts.wrtc === 'function' ? opts.wrtc() : opts.wrtc + // Use a socket pool, so WebSocket tracker clients share WebSocket objects for + // the same server. In practice, WebSockets are pretty slow to establish, so this + // gives a nice performance boost, and saves browser resources. + self._socketPool = {} + var announce = typeof opts.announce === 'string' ? [ opts.announce ] : opts.announce == null ? [] : opts.announce diff --git a/lib/client/websocket-tracker.js b/lib/client/websocket-tracker.js index 894408e..aa7e043 100644 --- a/lib/client/websocket-tracker.js +++ b/lib/client/websocket-tracker.js @@ -10,11 +10,6 @@ var Socket = require('simple-websocket') var common = require('../common') var Tracker = require('./tracker') -// Use a socket pool, so tracker clients share WebSocket objects for the same server. -// In practice, WebSockets are pretty slow to establish, so this gives a nice performance -// boost, and saves browser resources. -var socketPool = {} - var RECONNECT_MINIMUM = 15 * 1000 var RECONNECT_MAXIMUM = 30 * 60 * 1000 var RECONNECT_VARIANCE = 30 * 1000 @@ -129,15 +124,15 @@ WebSocketTracker.prototype.destroy = function (cb) { self._onSocketDataBound = null self._onSocketCloseBound = null - if (socketPool[self.announceUrl]) { - socketPool[self.announceUrl].consumers -= 1 + if (self.client._socketPool[self.announceUrl]) { + self.client._socketPool[self.announceUrl].consumers -= 1 } // Other instances are using the socket, so there's nothing left to do here - if (socketPool[self.announceUrl].consumers > 0) return cb() + if (self.client._socketPool[self.announceUrl].consumers > 0) return cb() - var socket = socketPool[self.announceUrl] - delete socketPool[self.announceUrl] + var socket = self.client._socketPool[self.announceUrl] + delete self.client._socketPool[self.announceUrl] socket.on('error', noop) // ignore all future errors socket.once('close', cb) @@ -182,11 +177,11 @@ WebSocketTracker.prototype._openSocket = function () { self._onSocketClose() } - self.socket = socketPool[self.announceUrl] + self.socket = self.client._socketPool[self.announceUrl] if (self.socket) { - socketPool[self.announceUrl].consumers += 1 + self.client._socketPool[self.announceUrl].consumers += 1 } else { - self.socket = socketPool[self.announceUrl] = new Socket(self.announceUrl) + self.socket = self.client._socketPool[self.announceUrl] = new Socket(self.announceUrl) self.socket.consumers = 1 self.socket.once('connect', self._onSocketConnectBound) }