diff options
-rw-r--r-- | CHANGELOG.md | 28 | ||||
-rw-r--r-- | index.js | 48 | ||||
-rw-r--r-- | lib/file.js | 6 | ||||
-rw-r--r-- | lib/rarity-map.js | 2 | ||||
-rw-r--r-- | lib/torrent.js | 33 | ||||
-rw-r--r-- | test/rarity-map.js | 2 |
6 files changed, 88 insertions, 31 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e24ca5..a5a5c07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,16 +25,36 @@ - Deprecate: Do not use `torrent.swarm` anymore. Use `torrent` instead. +- Only pass `torrent.infoHash` to the Chunk Store constructor, instead of the `Torrent` + instance itself, to prevent accidental memory leaks of the `Torrent` object by the + store. (Open an issue if you were using other properties. They can be re-added.) + +- Non-fatal errors with a single torrent will be emitted at `torrent.on('error')`. You + should listen to this event. Previously, all torrent errors were also emitted on + `client.on('error')` and handling `torrent.on('error')` was optional. This design is + better since now it is possible to distinguish between fatal client errors + (`client.on('error')`) when the whole client becomes unusable versus recoverable errors + where only a single torrent fails (`torrent.on('error')`) but the client can continue to + be used. However, if there is no `torrent.on('error')` event, then the error will be + forwarded to `client.on('error')`. This prevents crashing the client when the user + only has a listener on the client, but it makes it impossible for them to determine + a client error versus a torrent error. + ### Fixed -- When there is a `torrent.on('error')` listener, don't also emit - `client.on('error')`. +- If `client.get` is passed a `Torrent` instance, it now only returns it if it is present + in the client. + +- Errors creating a torrent with `client.seed` are now emitted on the returned `torrent` + object instead of the client (unless there is no event listeners on + `torrent.on('error')` as previously discussed). The torrent object is now also destroyed + automatically for the user, as was probably expected. - Do not return existing torrent object when duplicate torrent is added. Fire an `'error'` event instead. -- Memory leaks of `Torrent` object caused by various internal subclasses of WebTorrent, - including `RarityMap`, `TCPPool`, `WebConn`, `Server`. +- Memory leaks of `Torrent` object caused by many internal subclasses of WebTorrent, + including `RarityMap`, `TCPPool`, `WebConn`, `Server`, `File`. - `client.ratio` and `torrent.ratio` are now calculated as `uploaded / received` instead of `uploaded / downloaded`. @@ -93,8 +93,7 @@ function WebTorrent (opts) { // use a single DHT instance for all torrents, so the routing table can be reused self.dht = new DHT(extend({ nodeId: self.nodeId }, opts.dht)) self.dht.once('error', function (err) { - self.emit('error', err) - self._destroy() + self._destroy(err) }) // Ignore warning when there are > 10 torrents in the client @@ -171,17 +170,25 @@ Object.defineProperty(WebTorrent.prototype, 'ratio', { */ WebTorrent.prototype.get = function (torrentId) { var self = this - if (torrentId instanceof Torrent) return torrentId + var i, torrent + var len = self.torrents.length - var parsed - try { parsed = parseTorrent(torrentId) } catch (err) {} + if (torrentId instanceof Torrent) { + for (i = 0; i < len; i++) { + torrent = self.torrents[i] + if (torrent === torrentId) return torrent + } + } else { + var parsed + try { parsed = parseTorrent(torrentId) } catch (err) {} - if (!parsed) return null - if (!parsed.infoHash) throw new Error('Invalid torrent identifier') + if (!parsed) return null + if (!parsed.infoHash) throw new Error('Invalid torrent identifier') - for (var i = 0, len = self.torrents.length; i < len; i++) { - var torrent = self.torrents[i] - if (torrent.infoHash === parsed.infoHash) return torrent + for (i = 0; i < len; i++) { + torrent = self.torrents[i] + if (torrent.infoHash === parsed.infoHash) return torrent + } } return null } @@ -209,6 +216,7 @@ WebTorrent.prototype.add = function (torrentId, opts, ontorrent) { self.torrents.push(torrent) torrent.once('infoHash', function () { + if (self.destroyed) return for (var i = 0, len = self.torrents.length; i < len; i++) { var t = self.torrents[i] if (t.infoHash === torrent.infoHash && t !== torrent) { @@ -257,20 +265,20 @@ WebTorrent.prototype.seed = function (input, opts, onseed) { else cb(null, item) } }), function (err, input) { - if (err) return self.emit('error', err, torrent) + if (err) return torrent._destroy(err) if (self.destroyed) return createTorrent.parseInput(input, opts, function (err, files) { - if (err) return self.emit('error', err, torrent) + if (err) return torrent._destroy(err) if (self.destroyed) return streams = files.map(function (file) { return file.getStream }) createTorrent(input, opts, function (err, torrentBuf) { - if (err) return self.emit('error', err, torrent) + if (err) return torrent._destroy(err) if (self.destroyed) return var existingTorrent = self.get(torrentBuf) if (existingTorrent) { - torrent.destroy() + torrent._destroy(new Error('Cannot add duplicate torrent ' + torrent.infoHash)) _onseed(existingTorrent) } else { torrent._onTorrentId(torrentBuf) @@ -292,7 +300,7 @@ WebTorrent.prototype.seed = function (input, opts, onseed) { } parallel(tasks, function (err) { if (self.destroyed) return - if (err) return self.emit('error', err, torrent) + if (err) return torrent._destroy(err) _onseed(torrent) }) } @@ -313,10 +321,14 @@ WebTorrent.prototype.seed = function (input, opts, onseed) { */ WebTorrent.prototype.remove = function (torrentId, cb) { debug('remove') - var torrent = this.get(torrentId) if (!torrent) throw new Error('No torrent with id ' + torrentId) + this._remove(torrentId, cb) +} +WebTorrent.prototype._remove = function (torrentId, cb) { + var torrent = this.get(torrentId) + if (!torrent) return this.torrents.splice(this.torrents.indexOf(torrent), 1) torrent.destroy(cb) } @@ -363,6 +375,10 @@ WebTorrent.prototype._destroy = function (err, cb) { parallel(tasks, cb) if (err) self.emit('error', err) + + self.torrents = [] + self._tcpPool = null + self.dht = null } WebTorrent.prototype._onListening = function () { diff --git a/lib/file.js b/lib/file.js index bfa032e..c73fc38 100644 --- a/lib/file.js +++ b/lib/file.js @@ -1,5 +1,3 @@ -// TODO: cleanup reference to torrent (i.e. Torrent object) - module.exports = File var eos = require('end-of-stream') @@ -89,3 +87,7 @@ File.prototype.renderTo = function (elem, cb) { if (typeof window === 'undefined') throw new Error('browser-only method') render.render(this, elem, cb) } + +File.prototype._destroy = function () { + this._torrent = null +} diff --git a/lib/rarity-map.js b/lib/rarity-map.js index c362077..430f62f 100644 --- a/lib/rarity-map.js +++ b/lib/rarity-map.js @@ -3,8 +3,6 @@ module.exports = RarityMap /** * Mapping of torrent pieces to their respective availability in the torrent swarm. Used * by the torrent manager for implementing the rarest piece first selection strategy. - * - * @param {Torrent} torrent */ function RarityMap (torrent) { var self = this diff --git a/lib/torrent.js b/lib/torrent.js index fc5f495..296f579 100644 --- a/lib/torrent.js +++ b/lib/torrent.js @@ -92,10 +92,10 @@ function Torrent (torrentId, client, opts) { this.metadata = null this.store = null - this.files = null + this.files = [] + this.pieces = [] this._amInterested = false - this.pieces = [] this._selections = [] this._critical = [] @@ -253,6 +253,7 @@ Torrent.prototype._onParsedTorrent = function (parsedTorrent) { if (self._rechokeIntervalId.unref) self._rechokeIntervalId.unref() self.emit('infoHash', self.infoHash) + if (self.destroyed) return // user might destroy torrent in `infoHash` event handler if (self.client.listening) { self._onListening() @@ -371,7 +372,9 @@ Torrent.prototype._onMetadata = function (metadata) { self.store = new ImmediateChunkStore( new self._store(self.pieceLength, { - torrent: self, + torrent: { + infoHash: self.infoHash + }, files: self.files.map(function (file) { return { path: path.join(self.path, file.path), @@ -464,6 +467,7 @@ Torrent.prototype._verifyPieces = function () { var self = this parallelLimit(self.pieces.map(function (_, index) { return function (cb) { + if (self.destroyed) return cb(new Error('torrent is destroyed')) self.store.get(index, function (err, buf) { if (err) return cb(null) // ignore error sha1(buf, function (hash) { @@ -523,7 +527,7 @@ Torrent.prototype._destroy = function (err, cb) { self.destroyed = true self._debug('destroy') - self.client.remove(self) + self.client._remove(self) clearInterval(self._rechokeIntervalId) @@ -535,6 +539,10 @@ Torrent.prototype._destroy = function (err, cb) { self.removePeer(id) } + self.files.forEach(function (file) { + if (file instanceof File) file._destroy() + }) + var tasks = self._servers.map(function (server) { return function (cb) { server.destroy(cb) @@ -546,6 +554,7 @@ Torrent.prototype._destroy = function (err, cb) { self.discovery.destroy(cb) }) } + if (self.store) { tasks.push(function (cb) { self.store.close(cb) @@ -555,14 +564,26 @@ Torrent.prototype._destroy = function (err, cb) { parallel(tasks, cb) if (err) { - // When there is no `torrent.on('error')` listener, emit `client.on('error')` instead. - // The more-specific, torrent error handler is preferred. + // Torrent errors are emitted at `torrent.on('error')`. If there are no 'error' event + // handlers on the torrent instance, the error will be emitted at + // `client.on('error')`. This prevents crashing the user's program, but it makes it + // impossible to determine a client error versus a torrent error (where the client + // is still usable afterwards). Users are recommended for errors in both places + // to distinguish between the error types. if (self.listenerCount('error') === 0) { self.client.emit('error', err) } else { self.emit('error', err) } } + + self.client = null + self.files = [] + self.discovery = null + self.store = null + self._rarityMap = null + self._peers = null + self._servers = null } Torrent.prototype.addPeer = function (peer) { diff --git a/test/rarity-map.js b/test/rarity-map.js index 7a630ad..f7da99b 100644 --- a/test/rarity-map.js +++ b/test/rarity-map.js @@ -18,7 +18,7 @@ test('Rarity map usage', function (t) { torrentPort: 6889, dht: false, tracker: false, - remove: function () {} + _remove: function () {} } var opts = {} var torrent = new Torrent(torrentId, client, opts) |