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

Potential deadlock in SpitCase impl Stream #930

Closed
clux opened this issue Jun 8, 2022 · 2 comments · Fixed by #931
Closed

Potential deadlock in SpitCase impl Stream #930

clux opened this issue Jun 8, 2022 · 2 comments · Fixed by #931
Assignees
Labels
runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Jun 8, 2022

interesting new clippy lint seems to suggest that impl Stream for SplitCase can deadlock (rust issue: rust-lang/rust#93883 ).

this is used in applier.

haven't had time to look at it fully, but it looks similar to our setup, and it blocks ci atm.

clippied code shows this.inner being locked
https://github.com/kube-rs/kube-rs/blob/98cf1c89c37bb8bfc12b0d31d0b3264b13076771/kube-runtime/src/utils/mod.rs#L78-L97

@clux clux added the runtime controller runtime related label Jun 8, 2022
@clux
Copy link
Member Author

clux commented Jun 8, 2022

actually, it looks like we are doing the right thing here? we are locking in the base of that fn. this is invalid, right?
maybe clippy is getting confused by all the shadowing of inner?

@nightkr
Copy link
Member

nightkr commented Jun 8, 2022

Whoa, that's a thoroughly unhelpful error message.. :/

I don't think we have an issue here, since the temporaries involved here are:

  1. inner.as_mut(): just a "dumb" projection of inner, misuse is protected against since it grabs a mut borrow on inner for its lifetime
  2. poll_next(): stateful and complex, but the return value is "just" the ADT Poll<Option<S::Item>>

Poll and Option are both just dumb data and I don't see any has_significant_drop attribute on Pin, so I can only assume that it assumes the worst for S::Item?

nightkr added a commit to nightkr/kube-rs that referenced this issue Jun 8, 2022
Fixes kube-rs#930

This should ensure that we get a compile error if any temporary is even borrowed
past the `poll_next()` call.
@nightkr nightkr self-assigned this Jun 8, 2022
@clux clux closed this as completed in #931 Jun 8, 2022
clux pushed a commit that referenced this issue Jun 8, 2022
#931)

Silence false positive significant_drop_in_scrutinee for SplitCase

Fixes #930

This should ensure that we get a compile error if any temporary is even borrowed
past the `poll_next()` call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime controller runtime related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants