Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Panic when using AsyncReadExt::copy_into with a body and a async_std::fs::File #72

Closed
justinrlle opened this issue Sep 4, 2019 · 6 comments · Fixed by #73
Closed

Panic when using AsyncReadExt::copy_into with a body and a async_std::fs::File #72

justinrlle opened this issue Sep 4, 2019 · 6 comments · Fixed by #73
Assignees
@sagebind
Labels
bug Something is borken

Comments

@justinrlle
Copy link

justinrlle commented Sep 4, 2019 •
edited

The following snippet panics at the copy_into part, from the futures::io::AsyncReadExt:

use async_std::{task, fs};
use futures::io::AsyncReadExt as _;

fn main() {
    task::block_on(async {
        let mut file = fs::File::create("index.html")
            .await
            .expect("File::create");
        let mut res = isahc::get_async("https://gnu.org")
            .await
            .expect("isahc::get_async");

        res.body_mut()
            .copy_into(&mut file)
            .await
            .expect("copy_into");
    })
}
backtrace: 
cargo run --bin to_file
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/to_file`
thread 'async-task-driver' panicked at 'Receiver::next_message called after `None`', src/libcore/option.rs:1166:5
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.35/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.35/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:200
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:214
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
   7: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:384
   8: rust_begin_unwind
             at src/libstd/panicking.rs:311
   9: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  10: core::option::expect_failed
             at src/libcore/option.rs:1166
  11: core::option::Option<T>::expect
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libcore/option.rs:345
  12: futures_channel::mpsc::Receiver<T>::next_message
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-channel-preview-0.3.0-alpha.18/src/mpsc/mod.rs:842
  13: <futures_channel::mpsc::Receiver<T> as futures_core::stream::Stream>::poll_next
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-channel-preview-0.3.0-alpha.18/src/mpsc/mod.rs:912
  14: <sluice::pipe::chunked::Reader as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/sluice-0.4.1/src/pipe/chunked.rs:83
  15: <sluice::pipe::PipeReader as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/sluice-0.4.1/src/pipe/mod.rs:43
  16: <&mut T as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-io-preview-0.3.0-alpha.18/src/lib.rs:352
  17: <isahc::handler::ResponseBodyReader as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/isahc-0.7.1/src/handler.rs:458
  18: <&mut T as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-io-preview-0.3.0-alpha.18/src/lib.rs:352
  19: <isahc::client::ResponseBody as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/isahc-0.7.1/src/client.rs:960
  20: <isahc::body::Body as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/isahc-0.7.1/src/body.rs:159
  21: <&mut T as futures_io::if_std::AsyncRead>::poll_read
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-io-preview-0.3.0-alpha.18/src/lib.rs:352
  22: <futures_util::io::buf_reader::BufReader<R> as futures_io::if_std::AsyncBufRead>::poll_fill_buf
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-preview-0.3.0-alpha.18/src/io/buf_reader.rs:188
  23: <futures_util::io::copy_buf_into::CopyBufInto<R,W> as core::future::future::Future>::poll
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-preview-0.3.0-alpha.18/src/io/copy_buf_into.rs:46
  24: <futures_util::io::copy_into::CopyInto<R,W> as core::future::future::Future>::poll
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-preview-0.3.0-alpha.18/src/io/copy_into.rs:32
  25: std::future::poll_with_tls_context::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  26: std::future::get_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:110
  27: std::future::poll_with_tls_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  28: to_file::main::{{closure}}
             at ./to_file.rs:14
  29: <std::future::GenFuture<T> as core::future::future::Future>::poll::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  30: std::future::set_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:78
  31: <std::future::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  32: <std::panic::AssertUnwindSafe<F> as core::future::future::Future>::poll
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/panic.rs:334
  33: <futures_util::future::catch_unwind::CatchUnwind<Fut> as core::future::future::Future>::poll::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-preview-0.3.0-alpha.18/src/future/catch_unwind.rs:29
  34: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/panic.rs:315
  35: std::panicking::try::do_call
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/panicking.rs:296
  36: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:80
  37: std::panicking::try
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/panicking.rs:275
  38: std::panic::catch_unwind
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/panic.rs:394
  39: <futures_util::future::catch_unwind::CatchUnwind<Fut> as core::future::future::Future>::poll
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-preview-0.3.0-alpha.18/src/future/catch_unwind.rs:29
  40: std::future::poll_with_tls_context::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  41: std::future::get_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:110
  42: std::future::poll_with_tls_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  43: async_std::task::pool::block_on::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:105
  44: <std::future::GenFuture<T> as core::future::future::Future>::poll::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  45: std::future::set_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:78
  46: <std::future::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  47: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libcore/future/future.rs:119
  48: std::future::poll_with_tls_context::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  49: std::future::get_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:110
  50: std::future::poll_with_tls_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:120
  51: async_std::task::pool::spawn_with_builder::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:210
  52: <std::future::GenFuture<T> as core::future::future::Future>::poll::{{closure}}
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  53: std::future::set_task_context
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:78
  54: <std::future::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/future.rs:42
  55: async_task::raw::RawTask<F,R,S,T>::run
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-1.0.0/src/raw.rs:466
  56: async_task::task::Task<T>::run
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-1.0.0/src/task.rs:131
  57: <async_std::task::pool::spawn_with_builder::QUEUE as core::ops::deref::Deref>::deref::__static_ref_initialize::{{closure}}::{{closure}}::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:183
  58: async_std::task::pool::abort_on_panic
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:257
  59: <async_std::task::pool::spawn_with_builder::QUEUE as core::ops::deref::Deref>::deref::__static_ref_initialize::{{closure}}::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:183
  60: std::thread::local::LocalKey<T>::try_with
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/thread/local.rs:262
  61: std::thread::local::LocalKey<T>::with
             at /rustc/53df91a9b24ad999e7ca896447af6f5f74fe43bc/src/libstd/thread/local.rs:239
  62: <async_std::task::pool::spawn_with_builder::QUEUE as core::ops::deref::Deref>::deref::__static_ref_initialize::{{closure}}
             at /home/justin/.cargo/registry/src/github.com-1ecc6299db9ec823/async-std-0.99.4/src/task/pool.rs:180
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

It also panics if I copy_into the data to a async_std::io::Stdout:

use async_std::{task, io};
use futures::io::AsyncReadExt as _;

// panics
fn main() {
    task::block_on(async {
        let mut stdout = io::stdout();
        let mut res = isahc::get_async("https://gnu.org")
            .await
            .expect("isahc::get_async");

        res.body_mut()
            .copy_into(&mut stdout)
            .await
            .expect("copy_into");
    })
}

But if I pipe the data inside a async_std::io::sink, it doesn't panic:

use async_std::{task, io};
use futures::io::AsyncReadExt as _;

// does not panic
fn main() {
    task::block_on(async {
        let mut sink = io::sink();
        let mut res = isahc::get_async("https://gnu.org")
            .await
            .expect("isahc::get_async");

        res.body_mut()
            .copy_into(&mut sink)
            .await
            .expect("copy_into");
    })
}

It doens't panic either if I pipe inside a async_std::net::TcpStream:

use async_std::{task, net};
use futures::io::AsyncReadExt as _;

// does not panic
fn main() {
    task::block_on(async {
        let mut stream = net::TcpStream::connect("gnu.org:80")
            .await
            .expect("TcpStream::connect");
        let mut res = isahc::get_async("https://gnu.org")
            .await
            .expect("isahc::get_async");

        res.body_mut()
            .copy_into(&mut stream)
            .await
            .expect("copy_into");
    })
}

async_std::fs::File and async_std::io::Stdout have one thing in common: they use blocking io in a threadpool. But if I copy_into from a File to another, there is no panic:

use async_std::{task, fs};
use futures::io::AsyncReadExt as _;

// does not panic
fn main() {
    task::block_on(async {
        let mut file = fs::File::create("copy-Cargo.toml")
            .await
            .expect("File::create");

        fs::File::open("Cargo.toml")
            .await
            .expect("File::open")
            .copy_into(&mut file)
            .await
            .expect("copy_into");
    })
}

I've put up a gist with all the test cases, a Cargo.lock/Cargo.toml, and even a rust-toolchain file, to help with reproducing the issue: https://gist.github.com/justinrlle/ac3d9a44c50bc01a5f7f373cea93a1f0
@sagebind
Copy link
Owner

sagebind commented Sep 4, 2019

Thank you for your very comprehensive bug report! This does look like a problem. I will investigate this and work on a fix as soon as I can.

@sagebind sagebind added the bug Something is borken label Sep 4, 2019
@sagebind sagebind self-assigned this Sep 4, 2019
@sagebind
Copy link
Owner

sagebind commented Sep 4, 2019

Just taking a wild guess as to what the problem is, what happens if you call .fuse() right after .copy_into() and before awaiting?

@justinrlle
Copy link
Author

I just tried it, and unfortunately, it produces the same result. I feel like it might be a bug in async-std, or maybe even futures-preview, I'll try to look further.

@sagebind sagebind mentioned this issue Sep 4, 2019
Closed
@sagebind
Copy link
Owner

sagebind commented Sep 4, 2019

Ultimately something is calling poll_read() on the response body again after poll_read() previously returned Poll::Ready(Ok(0)). I think panicking in this situation is inappropriate... what's the correct behavior when trying to read after an EOF?

My guess is that futures_util::io::copy_into::CopyInto is doing something odd, like attempting another read after already receiving the EOF. Regardless, it's probably appropriate to fix sluice to continue returning EOF after it is closed, instead of allowing this panic. Opened upstream bug: sagebind/sluice#6

sagebind added a commit that referenced this issue Sep 5, 2019
@sagebind
Pull in read-after-EOF bugfix in Sluice
7d5b899 
Fixes #72.
@sagebind sagebind mentioned this issue Sep 5, 2019
Merged
sagebind added a commit that referenced this issue Sep 5, 2019
@sagebind
Pull in read-after-EOF bugfix in Sluice (#73)
bc2760f 
Fixes #72.
@sagebind
Copy link
Owner

sagebind commented Sep 5, 2019

This fix has been published in version 0.7.2!

@justinrlle
Copy link
Author

Thanks, I've just checked it, and it indeed works! Thanks for the fast reply and fix!

@sagebind sagebind mentioned this issue Jan 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sagebind sagebind

Labels
bug Something is borken
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pull in read-after-EOF bugfix in Sluice sagebind/isahc
2 participants
@sagebind @justinrlle