Open MattiasBuelens opened 3 years ago
I encountered same error and wonder it can occur actual bad effect or not. I also was trying to save Future
in RefCell.
Here is MRE which I could not make a panic.
use futures::stream::FuturesUnordered;
use futures::{Future, StreamExt};
use std::cell::RefCell;
async fn hello() {
tokio::time::sleep(std::time::Duration::from_micros(1));
}
struct Me<T: Future> {
queue: RefCell<FuturesUnordered<T>>,
}
impl<T: Future> Me<T> {
fn push(&mut self, t: T) {
self.queue.borrow_mut().push(t);
}
async fn next(&mut self) -> Option<T::Output> {
self.queue.borrow_mut().next().await
}
}
#[tokio::main]
async fn main() {
let mut me = Me {
queue: RefCell::new(FuturesUnordered::new()),
};
me.push(tokio::task::spawn(hello()));
loop {
tokio::select! {
Some(_) = me.next() => {
for _ in 0..100 {
me.push(tokio::task::spawn(hello()));
}
},
else => break
}
}
}
Honestly, I could not understand how exactly "holding the RefMut at await" is harmful. Is it coming from the fact that when async fn yields, they save their captured variables (so as RefMut
) and it may cause other async task touch the RefCell
again?
If then how the code above did not panic?
There isn't anything harmful about it, as long as you are doing it intentionally. Presumably the lint was added because people often do it by mistake when they really want something else.
I just came here from Google due to also getting a false alarm from this lint in a situation where holding RefMut across await is necessary but the structure of the code makes panics impossible (due to being guarded by an upstream &mut like in your example).
Note that in your example, you should be able to avoid the problem by changing the borrow_mut()s to get_mut()s, but this was not possible in my situation.
Lint name: await_holding_refcell_ref
I tried this code:
(For more context, see the complete code.)
This might seem a bit of a contrived example, but it's actually the only way I found to use
async fn
inside a Rust struct exported to JavaScript with wasm-bindgen. 🤷 (Better suggestions are welcome though!)I expected to see this happen: No error since the
RefMut
is necessary to keep theinner.write(chunk)
future alive.Instead, this happened: Causes a warning.
Meta
cargo clippy -V
: clippy 0.0.212 (e1884a8e 2020-12-29)rustc -Vv
:I noticed that this particular lint was already downgraded in #6354 for Rust 1.50 (currently in beta). I can confirm that the warning does not show up when using Rust beta or nightly with the default settings. Since it only shows up in Rust 1.49, I worked around it on my end by disabling this lint.
Still, I opened this issue since I think this might be an interesting false positive. Feel free to close if you disagree, no hard feelings. 🙂