More thorough object cleanup

- 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.

- 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.

- If `client.get` is passed a `Torrent` instance, it now only returns
it if it is present
  in the client.

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`.

index.js

@@ -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 () {

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

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

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

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)