tokio
tokio copied to clipboard
tokio-rs
`BufReader<impl AsyncWrite + AsyncSeek>` panics when calling `write_all` and immediately `seek`
Version tokio v1.19.2 and v1.20.1 tokio-util v0.7.3
Platform macOS 12.3 x86_64
Description The following code panics:
use std::io::SeekFrom;
use tokio::fs::File;
use tokio::io::{self, AsyncSeekExt, AsyncWriteExt, BufReader};
#[tokio::main]
async fn main() -> io::Result<()> {
let mut x = BufReader::new(File::create("test").await?);
x.write_all(b"test").await?;
x.seek(SeekFrom::Start(0)).await?;
Ok(())
}
thread 'main' panicked at 'must wait for poll_complete before calling start_seek', /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/fs/file.rs:570:28
stack backtrace:
0: std::panicking::begin_panic
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/panicking.rs:616:12
1: <tokio::fs::file::File as tokio::io::async_seek::AsyncSeek>::start_seek
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/fs/file.rs:570:28
2: <tokio::io::util::buf_reader::BufReader<R> as tokio::io::async_seek::AsyncSeek>::poll_complete
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/io/util/buf_reader.rs:242:17
3: <&mut T as tokio::io::async_seek::AsyncSeek>::poll_complete
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/io/async_seek.rs:56:13
4: <tokio::io::seek::Seek<S> as core::future::future::Future>::poll
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/io/seek.rs:49:25
5: test_mlua::main::{{closure}}
at ./src/main.rs:9:31
6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/future/mod.rs:91:19
7: tokio::park::thread::CachedParkThread::block_on::{{closure}}
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/park/thread.rs:263:54
8: tokio::coop::with_budget::{{closure}}
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/coop.rs:102:9
9: std::thread::local::LocalKey<T>::try_with
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/local.rs:445:16
10: std::thread::local::LocalKey<T>::with
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/std/src/thread/local.rs:421:9
11: tokio::coop::with_budget
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/coop.rs:95:5
12: tokio::coop::budget
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/coop.rs:72:5
13: tokio::park::thread::CachedParkThread::block_on
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/park/thread.rs:263:31
14: tokio::runtime::enter::Enter::block_on
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/runtime/enter.rs:152:13
15: tokio::runtime::thread_pool::ThreadPool::block_on
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/runtime/thread_pool/mod.rs:90:9
16: tokio::runtime::Runtime::block_on
at /Users/hacker/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.20.0/src/runtime/mod.rs:484:43
17: test_tokio::main
at ./src/main.rs:10:5
18: core::ops::function::FnOnce::call_once
at /rustc/e092d0b6b43f2de967af0887873151bb1c0b18d3/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
One way to fix this is to have BufReader flush the IO resource before attempting to seek.
The documentation for AsyncSeek specifies that an error should be returned if the seek cannot be started. It is incorrect for File to panic in this situation.
I was looking into this issue and opened a PR at #4897 to change the panic to an error.
Although the File impl seems incorrect, the behavior between BufReader and BufWriter's poll_complete impl is not consistent, i.e. BufWriter flushes its buffer before seeking:
https://github.com/tokio-rs/tokio/blob/9d9488db67136651f85d839efcb5f61aba8531c9/tokio/src/io/util/buf_writer.rs#L244-L245
Do you think it makes sense to also change BufReader to flush the underlying writer before attempting to seek? It would probably make the error in the snippet provided by @hack3ric less surprising
Actually, I realize now that BufReader will not be able to flush before seeking. We don't require AsyncWrite for AsyncSeek to be implemented on BufReader. If we make any further changes, it should probably be making File::start_seek work even if there is an active operation (other than a seek, that is).
Looking into File::start_seek I don't see an obvious change that would make the function work on a File busy with read / write. Conceptually, the seek operation needs to be queued but File's state only has a single slot that's already occupied by the pending operation.
Even if the JoinHandle in State::Busy was tagged with the type of pending operation (i.e. make it available outside the running blocking task), we wouldn't be able to start the seek and appropriately communicate that to the caller - we'd still have to return an error that we were unable to submit the operation
If we were to do that, then we would need to add an additional field to Busy to track that a seek should be started whenever the currently busy operation finishes. The poll_complete method (and probably poll_flush too) should then wait for the busy operation to complete, then submit the seek operation, then wait for the seek operation to complete.
It looks like the PR above (https://github.com/tokio-rs/tokio/pull/4897) has been merged. When I run the sample code from the initial post against version 1.21.0, it now comes back as an error result from the seek line, instead of a panic. Is this issue considered fixed?
