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

futures::sink::unfold easily causes panics if function returns errors #2600

Closed
ijackson opened this issue May 18, 2022 · 1 comment
Closed
Labels
A-sink Area: futures::sink

Comments

@ijackson
Copy link

I found some strange behaviour and have minimised it to this:

    #[derive(Debug,Eq,PartialEq)]
    struct TestError(char);

    #[async_test]
    async fn unfold_crash() {
        let mut unfold = Box::pin(futures::sink::unfold((), |(), c| async move {
            Err(TestError(c))
        }));
        for v in ['1','2','3'] {
            let _ = dbg!(unfold.send(v).await);
        }
    }

Which crashes:

---- futures::test::unfold_crash stdout ----
[crates/tor-basic-utils/src/futures.rs:158] unfold.send(v).await = Err(
    TestError(
        '1',
    ),
)
thread 'futures::test::unfold_crash' panicked at '`async fn` resumed after completion', crates/tor-basic-utils/src/futures.rs:154:80

I think what is happening is that (from the outside) the Sink impl for Unfold panics in some situations when it is called after returning an error. (The Sink trait docs say "In most cases, if the sink encounters an error, the sink will permanently be unable to receive items" which I think suggests it might keep returning Err, but doesn't seem to me to encompass panicking.)

On the inside, evidently it is trying to reuse a returned future R, after it got Ready(Err), when it ought to call F again to get a fresh future.

erebe added a commit to erebe/futures-rs that referenced this issue Jan 14, 2023
  - fix issue rust-lang#2600. When an Unfold sink return an error
  it is left in an invalid state. Calling after that flush/close
  cause the sink to panic due to re-calling a future already completed.

  This patch aims to leave the sink in a valid state and allow calling
  flush/close without causing a panic.
taiki-e pushed a commit that referenced this issue Jan 15, 2023
- fix issue #2600. When an Unfold sink return an error
  it is left in an invalid state. Calling after that flush/close
  cause the sink to panic due to re-calling a future already completed.

  This patch aims to leave the sink in a valid state and allow calling
  flush/close without causing a panic.
@taiki-e
Copy link
Member

taiki-e commented Jan 15, 2023

Fixed in #2686

@taiki-e taiki-e closed this as completed Jan 15, 2023
@taiki-e taiki-e added the A-sink Area: futures::sink label Jan 15, 2023
taiki-e pushed a commit that referenced this issue Jan 30, 2023
- fix issue #2600. When an Unfold sink return an error
  it is left in an invalid state. Calling after that flush/close
  cause the sink to panic due to re-calling a future already completed.

  This patch aims to leave the sink in a valid state and allow calling
  flush/close without causing a panic.
taiki-e pushed a commit that referenced this issue Jan 30, 2023
- fix issue #2600. When an Unfold sink return an error
  it is left in an invalid state. Calling after that flush/close
  cause the sink to panic due to re-calling a future already completed.

  This patch aims to leave the sink in a valid state and allow calling
  flush/close without causing a panic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sink Area: futures::sink
Projects
None yet
Development

No branches or pull requests

2 participants