ikatson / rqbit

A bittorrent client in Rust
Other
679 stars 61 forks source link

Calling blocking function in streaming? #153

Closed izderadicka closed 4 weeks ago

izderadicka commented 1 month ago

Hi,

first thanks for great SW - I just started to play with it, but works fine, is simple and ergonomic.

This is rather a question - I was looking into implementation of file streaming - e.g. FileStream in rqbit/crates/librqbit/src/torrent_state/streaming.rs.

Filestream implements tokio's AsyncRead trait, and and within poll_read it calls this:

poll_try_io!(poll_try_io!(self.torrent.with_storage_and_file(
            self.file_id,
            |files, _fi| {
                files.pread_exact(self.file_id, self.position, buf)?;
                Ok::<_, anyhow::Error>(())
            }
        )));

Looking into code for files.pread_exact - it looks like this is blocking call for default FilesystemStorage calling blocking File IO from std::io.
Is this problem or am I missing something?

ikatson commented 1 month ago

Hey, you are right, it's a blocking call from async context.

Historically, it was done this way but was called using "spawn_blocking" to prevent executor blocking.

I don't quite remember why exactly I did it this way years ago, but it wasn't ever a real problem to bother changing.

In streaming though (which is relatively new), I probably just forgot to wrap it in "block_in_place".

Feel free to do it! Although I don't think realistically it would impact anything visibly unless your disk is super slow and there are many streaming tasks to block all or most executor threads, but still is a perfectly fine fix.