Skip to content

Commit

Permalink
Remove "previously_requested_pieces"
Browse files Browse the repository at this point in the history
In #66 from a posted stacktrace
it became clear that setting previously_requested_pieces may panic.

I found a place where it was not initialized - in "on_have" callback.

I wanted to fix that + make it less error-prone, however noticed that
previously_requested_pieces isn't used at all anyway, because its use
was removed during one of the recent refactorings.

As things seem to be working fine without it, just removed it to simplify
code.
  • Loading branch information
ikatson committed Jan 21, 2024
1 parent fb3ff76 commit 2a60ff2
Showing 1 changed file with 8 additions and 21 deletions.
29 changes: 8 additions & 21 deletions crates/librqbit/src/torrent_state/live/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ fn dummy_file() -> anyhow::Result<std::fs::File> {
.with_context(|| format!("error opening {}", DEVNULL))
}

fn make_piece_bitfield(lengths: &Lengths) -> BF {
BF::from_vec(vec![0; lengths.piece_bitfield_bytes()])
}

pub(crate) struct TorrentStateLocked {
// What chunks we have and need.
// If this is None, the torrent was paused, and this live state is useless, and needs to be dropped.
Expand Down Expand Up @@ -429,10 +433,7 @@ impl TorrentStateLive {
addr: checked_peer.addr,
on_bitfield_notify: Default::default(),
unchoke_notify: Default::default(),
locked: RwLock::new(PeerHandlerLocked {
i_am_choked: true,
previously_requested_pieces: BF::new(),
}),
locked: RwLock::new(PeerHandlerLocked { i_am_choked: true }),
requests_sem: Semaphore::new(0),
state: self.clone(),
tx,
Expand Down Expand Up @@ -493,10 +494,7 @@ impl TorrentStateLive {
addr,
on_bitfield_notify: Default::default(),
unchoke_notify: Default::default(),
locked: RwLock::new(PeerHandlerLocked {
i_am_choked: true,
previously_requested_pieces: BF::new(),
}),
locked: RwLock::new(PeerHandlerLocked { i_am_choked: true }),
requests_sem: Semaphore::new(0),
state: state.clone(),
tx,
Expand Down Expand Up @@ -780,10 +778,6 @@ impl TorrentStateLive {

struct PeerHandlerLocked {
pub i_am_choked: bool,

// This is used to only request a piece from a peer once when stealing from others.
// So that you don't steal then re-steal the same piece in a loop.
pub previously_requested_pieces: BF,
}

// All peer state that would never be used by other actors should pe put here.
Expand Down Expand Up @@ -1117,8 +1111,7 @@ impl PeerHandler {
// If bitfield wasn't allocated yet, let's do it. Some clients start empty so they never
// send bitfields.
if live.bitfield.is_empty() {
live.bitfield =
BF::from_vec(vec![0; self.state.lengths.piece_bitfield_bytes()]);
live.bitfield = make_piece_bitfield(&self.state.lengths);
}
match live.bitfield.get_mut(have as usize) {
Some(mut v) => *v = true,
Expand All @@ -1128,8 +1121,8 @@ impl PeerHandler {
}
};
trace!("updated bitfield with have={}", have);
self.on_bitfield_notify.notify_waiters();
});
self.on_bitfield_notify.notify_waiters();
}

fn on_bitfield(&self, bitfield: ByteString) -> anyhow::Result<()> {
Expand All @@ -1140,7 +1133,6 @@ impl PeerHandler {
self.state.lengths.piece_bitfield_bytes(),
);
}
self.locked.write().previously_requested_pieces = BF::from_vec(vec![0; bitfield.len()]);
self.state
.peers
.update_bitfield_from_vec(self.addr, bitfield.0);
Expand Down Expand Up @@ -1229,11 +1221,6 @@ impl PeerHandler {
}
};

self.locked
.write()
.previously_requested_pieces
.set(next.get() as usize, true);

for chunk in self.state.lengths.iter_chunk_infos(next) {
let request = Request {
index: next.get(),
Expand Down

0 comments on commit 2a60ff2

Please sign in to comment.