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

repeated panics when using ServiceExt::oneshot #469

Closed
hdevalence opened this issue Sep 10, 2020 · 3 comments
Closed

repeated panics when using ServiceExt::oneshot #469

hdevalence opened this issue Sep 10, 2020 · 3 comments

Comments

@hdevalence
Copy link
Contributor

Using ServiceExt::oneshot (tower 0.3.1) I get repeated panics with the message "We immediately transition to ::Called".

It looks like this comes from the service impl:

impl<S, Req> Future for Oneshot<S, Req>
where
    S: Service<Req>,
{
    type Output = Result<S::Response, S::Error>;

    #[project]
    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        let mut this = self.project();
        loop {
            #[project]
            match this.state.as_mut().project() {
                State::NotReady(nr) => {
                    let (mut svc, req) = nr.take().expect("We immediately transition to ::Called");
                    let _ = ready!(svc.poll_ready(cx))?;
                    this.state.set(State::Called(svc.call(req)));
                }
                State::Called(fut) => {
                    let res = ready!(fut.poll(cx))?;
                    this.state.set(State::Done);
                    return Poll::Ready(Ok(res));
                }
                State::Done => panic!("polled after complete"),
            }
        }
    }
}

I'm not sure I follow the reasoning here: svc is taken from the nr: Option<_>, but if svc.poll_ready returns Poll::Pending, poll will return early (via ready) without putting svc back. So I don't see how it immediately transitions to ::Called.

@hawkw
Copy link
Member

hawkw commented Sep 10, 2020

I believe this is the same bug that was fixed in #447. Maybe that was not backported to 0.3?

hdevalence added a commit to ZcashFoundation/zebra that referenced this issue Sep 10, 2020
This is a really nice function but there might be a bug in its future
implementation: tower-rs/tower#469

This bug may have already been fixed for the 0.4.0 release, so we could change
back then.
hdevalence added a commit to ZcashFoundation/zebra that referenced this issue Sep 10, 2020
This is a really nice function but there might be a bug in its future
implementation: tower-rs/tower#469

This bug may have already been fixed for the 0.4.0 release, so we could change
back then.
hdevalence added a commit to ZcashFoundation/zebra that referenced this issue Sep 10, 2020
This is a really nice function but there might be a bug in its future
implementation: tower-rs/tower#469

This bug may have already been fixed for the 0.4.0 release, so we could change
back then.
hdevalence added a commit to ZcashFoundation/zebra that referenced this issue Sep 11, 2020
This is a really nice function but there might be a bug in its future
implementation: tower-rs/tower#469

This bug may have already been fixed for the 0.4.0 release, so we could change
back then.
@LucioFranco
Copy link
Member

@hdevalence I think you all updated to 0.4, is this still an issue or can we close this?

@hawkw
Copy link
Member

hawkw commented Jan 11, 2021

This is fixed in 0.4.

@hawkw hawkw closed this as completed Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants