From cf0d0118ea99e8492cf997e6d82b4105da4a3c76 Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Thu, 23 Jan 2020 13:54:37 +1100 Subject: [PATCH] Finish fixing IP banning on storage errors --- connection.go | 5 ++++- torrent.go | 17 ++++++++++------- torrent_test.go | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/connection.go b/connection.go index 76c15d32..4c2ebe53 100644 --- a/connection.go +++ b/connection.go @@ -1404,7 +1404,10 @@ func (c *connection) receiveChunk(msg *pp.Message) error { if t.pieceAllDirty(pieceIndex(req.Index)) { t.queuePieceCheck(pieceIndex(req.Index)) - t.pendAllChunkSpecs(pieceIndex(req.Index)) + // We don't pend all chunks here anymore because we don't want code dependent on the dirty + // chunk status (such as the haveChunk call above) to have to check all the various other + // piece states like queued for hash, hashing etc. This does mean that we need to be sure + // that chunk pieces are pended at an appropriate time later however. } cl.event.Broadcast() diff --git a/torrent.go b/torrent.go index 32db5146..f7f518f8 100644 --- a/torrent.go +++ b/torrent.go @@ -1532,8 +1532,8 @@ func (t *Torrent) SetMaxEstablishedConns(max int) (oldMax int) { return oldMax } -func (t *Torrent) pieceHashed(piece pieceIndex, correct bool) { - t.logger.Log(log.Fstr("hashed piece %d (passed=%t)", piece, correct).WithValues(debugLogValue)) +func (t *Torrent) pieceHashed(piece pieceIndex, passed bool, hashIoErr error) { + t.logger.Log(log.Fstr("hashed piece %d (passed=%t)", piece, passed).WithValues(debugLogValue)) p := t.piece(piece) p.numVerifies++ t.cl.event.Broadcast() @@ -1543,7 +1543,7 @@ func (t *Torrent) pieceHashed(piece pieceIndex, correct bool) { // Don't score the first time a piece is hashed, it could be an initial check. if p.storageCompletionOk { - if correct { + if passed { pieceHashedCorrect.Add(1) } else { log.Fmsg("piece %d failed hash: %d connections contributed", piece, len(p.dirtiers)).AddValues(t, p).Log(t.logger) @@ -1551,7 +1551,7 @@ func (t *Torrent) pieceHashed(piece pieceIndex, correct bool) { } } - if correct { + if passed { if len(p.dirtiers) != 0 { // Don't increment stats above connection-level for every involved connection. t.allStats((*ConnStats).incrementPiecesDirtiedGood) @@ -1564,14 +1564,17 @@ func (t *Torrent) pieceHashed(piece pieceIndex, correct bool) { if err != nil { t.logger.Printf("%T: error marking piece complete %d: %s", t.storage, piece, err) } + t.pendAllChunkSpecs(piece) } else { - if len(p.dirtiers) != 0 && p.allChunksDirty() { + if len(p.dirtiers) != 0 && p.allChunksDirty() && hashIoErr == nil { + // Peers contributed to all the data for this piece hash failure, and the failure was + // not due to errors in the storage (such as data being dropped in a cache). // Increment Torrent and above stats, and then specific connections. t.allStats((*ConnStats).incrementPiecesDirtiedBad) for c := range p.dirtiers { // Y u do dis peer?! - c._stats.incrementPiecesDirtiedBad() + c.stats().incrementPiecesDirtiedBad() } bannableTouchers := make([]*connection, 0, len(p.dirtiers)) @@ -1698,7 +1701,7 @@ func (t *Torrent) pieceHasher(index pieceIndex) { defer t.cl.unlock() p.hashing = false t.updatePiecePriority(index) - t.pieceHashed(index, correct) + t.pieceHashed(index, correct, copyErr) t.publishPieceChange(index) t.activePieceHashes-- t.tryCreateMorePieceHashers() diff --git a/torrent_test.go b/torrent_test.go index cdedad1e..5ecd93b4 100644 --- a/torrent_test.go +++ b/torrent_test.go @@ -157,7 +157,7 @@ func TestPieceHashFailed(t *testing.T) { tt.cl.lock() tt.pieces[1]._dirtyChunks.AddRange(0, 3) require.True(t, tt.pieceAllDirty(1)) - tt.pieceHashed(1, false) + tt.pieceHashed(1, false, nil) // Dirty chunks should be cleared so we can try again. require.False(t, tt.pieceAllDirty(1)) tt.cl.unlock() -- 2.48.1