fix: Cleanup duplicated deselect() code (#2113)

The fileStream._destroy() function would never run because when fileStream.destroy() is called then it sets a 'destroyed' getter to true, which shadows our own fileStream.destroyed property.

The code outside running eos() was reaching into fileStream internals, so I just moved it in there.

2 files changed, 23 insertions, 17 deletions

diff --git a/lib/file-stream.js b/lib/file-stream.js
index f9cd462..b499754 100644
--- a/lib/file-stream.js
+++ b/lib/file-stream.js
@@ -1,5 +1,6 @@
 const debug = require('debug')('webtorrent:file-stream')
 const stream = require('readable-stream')
+const eos = require('end-of-stream')
 
 /**
  * Readable stream of a torrent file
@@ -13,7 +14,6 @@ class FileStream extends stream.Readable {
   constructor (file, opts) {
     super(opts)
 
-    this.destroyed = false
     this._torrent = file._torrent
 
     const start = (opts && opts.start) || 0
@@ -33,6 +33,15 @@ class FileStream extends stream.Readable {
     this._reading = false
     this._notifying = false
     this._criticalLength = Math.min((1024 * 1024 / pieceLength) | 0, 2)
+
+    this._torrent.select(this._startPiece, this._endPiece, true, () => {
+      this._notify()
+    })
+
+    // Ensure that cleanup happens even if destroy() is never called (readable-stream v3 currently doesn't call it automaticallly)
+    eos(this, (err) => {
+      this.destroy(err)
+    })
   }
 
   _read () {
@@ -85,16 +94,11 @@ class FileStream extends stream.Readable {
     this._piece += 1
   }
 
-  _destroy (err) {
-    if (this.destroyed) return
-    this.destroyed = true
-
+  _destroy (err, cb) {
     if (!this._torrent.destroyed) {
       this._torrent.deselect(this._startPiece, this._endPiece, true)
     }
-
-    if (err) this.emit('error', err)
-    this.emit('close')
+    cb(err)
   }
 }
 
diff --git a/lib/file.js b/lib/file.js
index e39d5d7..fd9ac7c 100644
--- a/lib/file.js
+++ b/lib/file.js
@@ -1,6 +1,5 @@
 const { EventEmitter } = require('events')
 const { PassThrough } = require('readable-stream')
-const eos = require('end-of-stream')
 const path = require('path')
 const render = require('render-media')
 const streamToBlob = require('stream-to-blob')
@@ -15,6 +14,7 @@ class File extends EventEmitter {
 
     this._torrent = torrent
     this._destroyed = false
+    this._fileStreams = new Set()
 
     this.name = file.name
     this.path = file.path
@@ -103,15 +103,12 @@ class File extends EventEmitter {
     }
 
     const fileStream = new FileStream(this, opts)
-    this._torrent.select(fileStream._startPiece, fileStream._endPiece, true, () => {
-      fileStream._notify()
-    })
-    eos(fileStream, () => {
-      if (this._destroyed) return
-      if (!this._torrent.destroyed) {
-        this._torrent.deselect(fileStream._startPiece, fileStream._endPiece, true)
-      }
+
+    this._fileStreams.add(fileStream)
+    fileStream.once('close', () => {
+      this._fileStreams.delete(fileStream)
     })
+
     return fileStream
   }
 
@@ -154,6 +151,11 @@ class File extends EventEmitter {
   _destroy () {
     this._destroyed = true
     this._torrent = null
+
+    for (const fileStream of this._fileStreams) {
+      fileStream.destroy()
+    }
+    this._fileStreams.clear()
   }
 }