From 11ced941e39d9f4c6dd76701c2017a05468b9d65 Mon Sep 17 00:00:00 2001
From: Kaylee <34007889+KayleePop@users.noreply.github.com>
Date: Wed, 11 Apr 2018 18:22:03 -0500
Subject: [PATCH 1/5] Defer store verification

Allow file access before the entire store has been validated.

Critical pieces are validated immediately instead of waiting in the
parallelLimit() queue.
---
 lib/torrent.js | 91 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 69 insertions(+), 22 deletions(-)

diff --git a/lib/torrent.js b/lib/torrent.js
index c0e8aafc..1002b978 100644
--- a/lib/torrent.js
+++ b/lib/torrent.js
@@ -100,6 +100,8 @@ function Torrent (torrentId, client, opts) {
   this._amInterested = false
   this._selections = []
   this._critical = []
+  this._verified = []
+  this._verifying = []
 
   this.wires = [] // open wires (added *after* handshake)
 
@@ -548,7 +550,6 @@ Torrent.prototype._onMetadata = function (metadata) {
         for (var index = 0; index < self.pieces.length; index++) {
           self._markVerified(index)
         }
-        self._onStore()
       } else {
         self._verifyPieces()
       }
@@ -558,6 +559,7 @@ Torrent.prototype._onMetadata = function (metadata) {
   }
 
   self.emit('metadata')
+  self._onStore()
 }
 
 /*
@@ -586,30 +588,64 @@ 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 (self.destroyed) return cb(new Error('torrent is destroyed'))
-
-        if (err) return process.nextTick(cb, null) // ignore error
-        sha1(buf, function (hash) {
-          if (self.destroyed) return cb(new Error('torrent is destroyed'))
-
-          if (hash === self._hashes[index]) {
-            if (!self.pieces[index]) return
-            self._debug('piece verified %s', index)
-            self._markVerified(index)
-          } else {
-            self._debug('piece invalid %s', index)
-          }
-          cb(null)
-        })
-      })
+      self._verify(index, cb)
     }
   }), FILESYSTEM_CONCURRENCY, function (err) {
     if (err) return self._destroy(err)
     self._debug('done verifying')
-    self._onStore()
+  })
+}
+
+Torrent.prototype._verify = function (index, cb) {
+  var self = this
+
+  if (self.destroyed) return cb(new Error('torrent is destroyed'))
+  if (self._verifying[index] || self.bitfield.get(index) || self._verified[index]) {
+    // call cb() asynchronously so that parallelLimit() won't block
+    return process.nextTick(cb, null)
+  }
+
+  self._verifying[index] = true
+
+  self.store.get(index, function (err, buf) {
+    if (self.destroyed) return cb(new Error('torrent is destroyed'))
+    // a critical piece might have been downloaded before this callback
+    if (self.bitfield.get(index)) return cb(null)
+
+    if (err) {
+      // interpret any error as not having the chunk
+      self._verified[index] = true
+      self._update()
+      return cb(null)
+    }
+
+    sha1(buf, function (hash) {
+      if (self.destroyed) return cb(new Error('torrent is destroyed'))
+      // a critical piece might have been downloaded before this callback
+      if (self.bitfield.get(index)) return cb(null)
+
+      if (hash === self._hashes[index]) {
+        self._debug('piece verified %s', index)
+        self._markVerified(index)
+
+        self.wires.forEach(function (wire) {
+          wire.have(index)
+        })
+
+        // We also check `self.destroyed` since `torrent.destroy()` could have been
+        // called in the `torrent.on('done')` handler, triggered by `_checkDone()`.
+        if (self._checkDone() && !self.destroyed) self.discovery.complete()
+
+        self._update()
+
+        cb(null)
+      } else {
+        self._debug('piece invalid %s', index)
+        self._verified[index] = true
+        self._update()
+        cb(null)
+      }
+    })
   })
 }
 
@@ -1470,6 +1506,16 @@ Torrent.prototype._request = function (wire, index, hotswap) {
 
   if (self.bitfield.get(index)) return false
 
+  if (!self._verified[index]) {
+    if (self._critical[index]) {
+      // if the piece is critical then start its verification immediately
+      self._verify(index, noop)
+    } else {
+      // cancel the request unless the piece is critical
+      return false
+    }
+  }
+
   var maxOutstandingRequests = isWebSeed
     ? Math.min(
       getPiecePipelineLength(wire, PIPELINE_MAX_DURATION, self.pieceLength),
@@ -1498,7 +1544,8 @@ Torrent.prototype._request = function (wire, index, hotswap) {
   var chunkLength = isWebSeed ? piece.chunkLengthRemaining(reservation) : piece.chunkLength(reservation)
 
   wire.request(index, chunkOffset, chunkLength, function onChunk (err, chunk) {
-    if (self.destroyed) return
+    // A critical piece may have been verified as in the store before the callback
+    if (self.destroyed || self.bitfield.get(index)) return
 
     // TODO: what is this for?
     if (!self.ready) return self.once('ready', function () { onChunk(err, chunk) })

From 9249d80eb9002bbca140381dabe72a920bace817 Mon Sep 17 00:00:00 2001
From: Kaylee <34007889+KayleePop@users.noreply.github.com>
Date: Wed, 11 Apr 2018 18:24:25 -0500
Subject: [PATCH 2/5] rename _markVerified() to _markInStore()

This avoids confusion with setting an index in _verified[] to true.
---
 lib/torrent.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/torrent.js b/lib/torrent.js
index 1002b978..44b3e01a 100644
--- a/lib/torrent.js
+++ b/lib/torrent.js
@@ -548,7 +548,7 @@ Torrent.prototype._onMetadata = function (metadata) {
 
       if (unchanged) {
         for (var index = 0; index < self.pieces.length; index++) {
-          self._markVerified(index)
+          self._markInStore(index)
         }
       } else {
         self._verifyPieces()
@@ -626,7 +626,7 @@ Torrent.prototype._verify = function (index, cb) {
 
       if (hash === self._hashes[index]) {
         self._debug('piece verified %s', index)
-        self._markVerified(index)
+        self._markInStore(index)
 
         self.wires.forEach(function (wire) {
           wire.have(index)
@@ -649,7 +649,7 @@ Torrent.prototype._verify = function (index, cb) {
   })
 }
 
-Torrent.prototype._markVerified = function (index) {
+Torrent.prototype._markInStore = function (index) {
   this.pieces[index] = null
   this._reservations[index] = null
   this.bitfield.set(index, true)

From 35aeb52801aa69ad4b69b6e7f5b459d94b01fda0 Mon Sep 17 00:00:00 2001
From: Kaylee <34007889+KayleePop@users.noreply.github.com>
Date: Wed, 11 Apr 2018 18:26:37 -0500
Subject: [PATCH 3/5] Remove _onStore() function

This function isn't useful anymore because store verification happens
after the 'ready' event.
---
 lib/torrent.js | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/lib/torrent.js b/lib/torrent.js
index 44b3e01a..d54ee634 100644
--- a/lib/torrent.js
+++ b/lib/torrent.js
@@ -534,6 +534,8 @@ Torrent.prototype._onMetadata = function (metadata) {
     self._onWireWithMetadata(wire)
   })
 
+  self.emit('metadata')
+
   self._debug('verifying existing torrent data')
   if (self._fileModtimes && self._store === FSChunkStore) {
     // don't verify if the files haven't been modified since we last checked
@@ -550,6 +552,9 @@ Torrent.prototype._onMetadata = function (metadata) {
         for (var index = 0; index < self.pieces.length; index++) {
           self._markInStore(index)
         }
+        self._checkDone()
+
+        self.updateSelections()
       } else {
         self._verifyPieces()
       }
@@ -558,8 +563,8 @@ Torrent.prototype._onMetadata = function (metadata) {
     self._verifyPieces()
   }
 
-  self.emit('metadata')
-  self._onStore()
+  self.ready = true
+  self.emit('ready')
 }
 
 /*
@@ -655,24 +660,6 @@ Torrent.prototype._markInStore = function (index) {
   this.bitfield.set(index, true)
 }
 
-/**
- * Called when the metadata, listening server, and underlying chunk store is initialized.
- */
-Torrent.prototype._onStore = function () {
-  var self = this
-  if (self.destroyed) return
-  self._debug('on store')
-
-  self.ready = true
-  self.emit('ready')
-
-  // Files may start out done if the file was already in the store
-  self._checkDone()
-
-  // In case any selections were made before torrent was ready
-  self._updateSelections()
-}
-
 Torrent.prototype.destroy = function (cb) {
   var self = this
   self._destroy(null, cb)

From 40a67a5ba9c65325e68b647e30654b8b776ca52e Mon Sep 17 00:00:00 2001
From: Kaylee <34007889+KayleePop@users.noreply.github.com>
Date: Wed, 11 Apr 2018 18:29:34 -0500
Subject: [PATCH 4/5] Clarify debug and error messages regarding piece
 verification

Specify whether the verification was on a piece in the store, or on a piece from a wire.
---
 lib/torrent.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/torrent.js b/lib/torrent.js
index d54ee634..6fe477a5 100644
--- a/lib/torrent.js
+++ b/lib/torrent.js
@@ -630,7 +630,7 @@ Torrent.prototype._verify = function (index, cb) {
       if (self.bitfield.get(index)) return cb(null)
 
       if (hash === self._hashes[index]) {
-        self._debug('piece verified %s', index)
+        self._debug('piece from store verified %s', index)
         self._markInStore(index)
 
         self.wires.forEach(function (wire) {
@@ -645,7 +645,7 @@ Torrent.prototype._verify = function (index, cb) {
 
         cb(null)
       } else {
-        self._debug('piece invalid %s', index)
+        self._debug('piece from store invalid %s', index)
         self._verified[index] = true
         self._update()
         cb(null)
@@ -1568,7 +1568,7 @@ Torrent.prototype._request = function (wire, index, hotswap) {
 
       if (hash === self._hashes[index]) {
         if (!self.pieces[index]) return
-        self._debug('piece verified %s', index)
+        self._debug('piece from wire verified %s', index)
 
         self.pieces[index] = null
         self._reservations[index] = null
@@ -1585,7 +1585,7 @@ Torrent.prototype._request = function (wire, index, hotswap) {
         if (self._checkDone() && !self.destroyed) self.discovery.complete()
       } else {
         self.pieces[index] = new Piece(piece.length)
-        self.emit('warning', new Error('Piece ' + index + ' failed verification'))
+        self.emit('warning', new Error('Piece from wire ' + index + ' failed verification'))
       }
       onUpdateTick()
     })

From 5d1934b03f8848faa2518c0a7b955a153b732039 Mon Sep 17 00:00:00 2001
From: Kaylee <34007889+KayleePop@users.noreply.github.com>
Date: Mon, 18 Jun 2018 16:13:28 -0500
Subject: [PATCH 5/5] Don't download critical pieces before verification

This makes metadata always available before every request, and race
conditions between verification and downloading aren't possible.

Critical pieces are still prioritized for verification.
---
 lib/torrent.js | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/lib/torrent.js b/lib/torrent.js
index 6fe477a5..bfc0a1be 100644
--- a/lib/torrent.js
+++ b/lib/torrent.js
@@ -605,7 +605,7 @@ Torrent.prototype._verify = function (index, cb) {
   var self = this
 
   if (self.destroyed) return cb(new Error('torrent is destroyed'))
-  if (self._verifying[index] || self.bitfield.get(index) || self._verified[index]) {
+  if (self._verifying[index] || self._verified[index]) {
     // call cb() asynchronously so that parallelLimit() won't block
     return process.nextTick(cb, null)
   }
@@ -614,8 +614,6 @@ Torrent.prototype._verify = function (index, cb) {
 
   self.store.get(index, function (err, buf) {
     if (self.destroyed) return cb(new Error('torrent is destroyed'))
-    // a critical piece might have been downloaded before this callback
-    if (self.bitfield.get(index)) return cb(null)
 
     if (err) {
       // interpret any error as not having the chunk
@@ -626,8 +624,6 @@ Torrent.prototype._verify = function (index, cb) {
 
     sha1(buf, function (hash) {
       if (self.destroyed) return cb(new Error('torrent is destroyed'))
-      // a critical piece might have been downloaded before this callback
-      if (self.bitfield.get(index)) return cb(null)
 
       if (hash === self._hashes[index]) {
         self._debug('piece from store verified %s', index)
@@ -1494,13 +1490,10 @@ Torrent.prototype._request = function (wire, index, hotswap) {
   if (self.bitfield.get(index)) return false
 
   if (!self._verified[index]) {
-    if (self._critical[index]) {
-      // if the piece is critical then start its verification immediately
-      self._verify(index, noop)
-    } else {
-      // cancel the request unless the piece is critical
-      return false
-    }
+    // verify critical pieces as soon as possible
+    if (self._critical[index]) self._verify(index, noop)
+
+    return false
   }
 
   var maxOutstandingRequests = isWebSeed
@@ -1531,11 +1524,7 @@ Torrent.prototype._request = function (wire, index, hotswap) {
   var chunkLength = isWebSeed ? piece.chunkLengthRemaining(reservation) : piece.chunkLength(reservation)
 
   wire.request(index, chunkOffset, chunkLength, function onChunk (err, chunk) {
-    // A critical piece may have been verified as in the store before the callback
-    if (self.destroyed || self.bitfield.get(index)) return
-
-    // TODO: what is this for?
-    if (!self.ready) return self.once('ready', function () { onChunk(err, chunk) })
+    if (self.destroyed) return
 
     if (r[i] === wire) r[i] = null