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.

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

Lines::poll_next panics if the reader returns an error after returning data #2862

Closed
jstarks opened this issue May 30, 2024 · 1 comment · Fixed by #2884
Closed

Lines::poll_next panics if the reader returns an error after returning data #2862

jstarks opened this issue May 30, 2024 · 1 comment · Fixed by #2884
Labels
A-io Area: futures::io bug

Comments

@jstarks
Copy link

jstarks commented May 30, 2024

0.3.30:

Lines::poll_next propagates the error from reader before it restores the invariant that buf is empty. This results in panics on IO failures.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=488083df8fff1bae3dd22cabbb405451

use futures::AsyncRead;
use futures::StreamExt;
use futures::AsyncBufReadExt;
use futures::executor::block_on;
use std::task::Poll;

struct BadReader(bool);

impl AsyncRead for BadReader {
    fn poll_read(mut self: std::pin::Pin<&mut Self>, _cx: &mut std::task::Context<'_>, b: &mut [u8]) -> Poll<std::io::Result<usize>> {
        if self.0 {
            return Poll::Ready(Err(std::io::ErrorKind::InvalidInput.into()));
        } else {
            self.0 = true;
            b.fill(b'x');
            Ok(b.len()).into()
        }
    }
}

fn main() {
    let mut lines = futures::io::BufReader::new(BadReader(false)).lines();
    while let Some(_) = block_on(lines.next()) {}
}

Output:

  Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.66s
     Running `target/debug/playground`
thread 'main' panicked at /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/io/read_line.rs:44:9:
assertion `left == right` failed
  left: 8192
 right: 0
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/panicking.rs:645:5
   1: core::panicking::panic_fmt
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:72:14
   2: core::panicking::assert_failed_inner
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:343:17
   3: core::panicking::assert_failed
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/panicking.rs:298:5
   4: futures_util::io::read_line::read_line_internal
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/io/read_line.rs:44:9
   5: <futures_util::io::lines::Lines<R> as futures_core::stream::Stream>::poll_next
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/io/lines.rs:35:24
   6: futures_util::stream::stream::StreamExt::poll_next_unpin
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/mod.rs:1638:9
   7: <futures_util::stream::stream::next::Next<St> as core::future::future::Future>::poll
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/next.rs:32:9
   8: futures_executor::local_pool::block_on::{{closure}}
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-executor-0.3.30/src/local_pool.rs:317:23
   9: futures_executor::local_pool::run_executor::{{closure}}
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-executor-0.3.30/src/local_pool.rs:90:37
  10: std::thread::local::LocalKey<T>::try_with
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/thread/local.rs:284:16
  11: std::thread::local::LocalKey<T>::with
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/std/src/thread/local.rs:260:9
  12: futures_executor::local_pool::run_executor
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-executor-0.3.30/src/local_pool.rs:86:5
  13: futures_executor::local_pool::block_on
             at ./.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-executor-0.3.30/src/local_pool.rs:317:5
  14: playground::main
             at ./src/main.rs:23:25
  15: core::ops::function::FnOnce::call_once
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
@taiki-e taiki-e added bug A-io Area: futures::io labels May 31, 2024
@hkratz
Copy link
Contributor

hkratz commented Sep 13, 2024

AsyncBufReadExt::read_line has the same issue. I will open a PR which fixes this (and some other issues with read_line) shortly.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=a4a58745c2bfb7fe0c7fae4acdf317d7

hkratz added a commit to hkratz/futures-rs that referenced this issue Sep 14, 2024
Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

* Fixes the following issues in `AsyncBufRead::read_line`
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (rust-lang#2862)

Fixes rust-lang#2862.
hkratz added a commit to hkratz/futures-rs that referenced this issue Sep 14, 2024
Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

* Fixes the following issues in `AsyncBufRead::read_line`
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (rust-lang#2862)

Fixes rust-lang#2862.
taiki-e pushed a commit that referenced this issue Oct 5, 2024
#2884)

Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

Fixes the following issues in `AsyncBufRead::read_line`:
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (#2862)

Fixes #2862
taiki-e pushed a commit that referenced this issue Oct 5, 2024
#2884)

Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

Fixes the following issues in `AsyncBufRead::read_line`:
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (#2862)

Fixes #2862
taiki-e pushed a commit that referenced this issue Oct 5, 2024
#2884)

Fixes the following issues in `AsyncBufRead::read_line`:
* When the future is dropped the previous string contents are not restored so the string is empty.
* If invalid UTF-8 is encountered the previous string contents are not restored.
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails.
* Performance: The whole string to which read contents are appended is check for UTF-8 validity instead of just the added bytes.

Fixes the following issues in `AsyncBufRead::read_line`:
* If an IO error occurs after `read_until_internal` already read a couple of bytes a debug assertion fails. (#2862)

Fixes #2862
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: futures::io bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants