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

fix: dont use fuse::select_next_some #532

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Conversation

mattsse
Copy link
Member

@mattsse mattsse commented Apr 14, 2024

closes foundry-rs/foundry#7652

SelectNextSome over Fuse will panic

Fuse will return

        if *this.done {
            return Poll::Ready(None);
        }

but selectsome will panic if the underlying stream already returned None once

        if let Some(item) = ready!(self.stream.poll_next_unpin(cx)) {
            Poll::Ready(item)
        } else {
            debug_assert!(self.stream.is_terminated());
            cx.waker().wake_by_ref();
            Poll::Pending
        }

@mattsse
Copy link
Member Author

mattsse commented Apr 14, 2024

hmm, actually this is very weird, because the fuse stream is always marked as terminated when it returns None

which makes me think the issue is somewhere else

EDIT:

ah my bad looked at the wrong assert, it hits the first:

   fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        assert!(!self.stream.is_terminated(), "SelectNextSome polled after terminated");

        if let Some(item) = ready!(self.stream.poll_next_unpin(cx)) {
            Poll::Ready(item)
        } else {
            debug_assert!(self.stream.is_terminated());
            cx.waker().wake_by_ref();
            Poll::Pending
        }
    }

so this change fixes it

@mattsse mattsse merged commit 037dd4b into main Apr 14, 2024
18 checks passed
@mattsse mattsse deleted the matt/fix-select-next-some branch April 14, 2024 15:13
ben186 pushed a commit to ben186/alloy that referenced this pull request Jul 27, 2024
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

Successfully merging this pull request may close these issues.

forge create application crash
2 participants