I don't understand this lifetime + async quirk

dpc · February 7, 2023, 12:16am

I have a type that was not Send because:

    = help: the trait `Sync` is not implemented for `(dyn IPeerConnections<EpochMessage> + std::marker::Send + 'static)`
note: future is not `Send` as this value is used across an await
   --> /home/dpc/lab/fedimint.git/main/fedimint-server/src/lib.rs:318:25
    |
295 |           for epoch_num in self.next_epoch_to_process()..=last_outcome.epoch {
    |           -                ---- has type `&FedimintServer` which is not `Send`
    |  _________|
    | |
296 | |             let (items, epoch, prev_epoch_hash, rejected_txs, at_know_trusted_checkpoint) =
297 | |                 if epoch_num == last_outcome.epoch {
298 | |                     (
...   |
318 | |                         .await
    | |                         ^^^^^^ await occurs here, with `self` maybe used later
...   |
351 | |             }
352 | |         }
    | |_________- `self` is later dropped her

where this await is an a function:

    pub async fn process_outcome(
        &mut self,
        last_outcome: HbbftConsensusOutcome,
    ) -> Result<(), EpochVerifyError> {

which is holding a &mut, so should be exclusive access and ... Send.

That self.next_epoch_to_process() is taking &self, but returning u64, so should not be holding reference to &self after that call is done.

I finally figured out that the following fixes it:

-        for epoch_num in self.next_epoch_to_process()..=last_outcome.epoch {
+        let next_epoch_to_process = self.next_epoch_to_process();
+        for epoch_num in next_epoch_to_process..=last_outcome.epoch {

but I don't understand why. What borrowck feature/quirk am I missing?

In case the you'd like to see the whole code: permalink

steffahn · February 7, 2023, 2:26am

This is probably about the scope of temporary variables. The temporary variables in the expression evaluating the collection for a for loop will only be dropped at the end of the surrounding statement (i. e. in this case, after the loop) and there's also no smart detection in the compiler (yet?) that could figure out that is can actually be made to live for shorter, to avoid holding the reference over an await.

Edit on second though, I'm not 100% sure why there would be a temporary holding the reference in the first place. I don't feel like maybe there ought not to be a temporary at all.

Edit2 well, there technically always exist temporaries for all function arguments of a function, so that they get dropped if a later argument panics. If that's the origin of the problem here that seems like something that should definitely be improved upon (in the analysis of what could possible be held over an await point) since those temporaries are always moved out of before the call happens, which is way before the await could ever be reached. That being said, AFAIR, the same detection has a hard time in general reasoning about the fact that moved-out-of variables aren't "really" ever being held over an await point.

vague · February 7, 2023, 2:37am

I have no idea about it. Just to raise a simplified version I can see. And it unfortunately compiles. playground

gabhijit · February 7, 2023, 2:38am

Your &FedimintServer is not Send and hence it cannot be used across await points (think about awaiting in one iteration) and then coming again to for ...

This might help as you don't have to do self.next_epoch_to_process() in the for loop.

let next_epoch = self.next_epoch_to_proccess();

// We are done using `self` here and is no longer used in the for loop.

for epoch_num in next_epoch..=last_outcome { 
...
}

steffahn · February 7, 2023, 2:43am

Isn't S simply Send + Sync in this "simplified" code?

dpc · February 7, 2023, 2:46am

This is slightly incorrect. I'll try to make it like the real code. Thanks!

dpc · February 7, 2023, 3:07am

OK, so I was confused in one important way. It's not that this sturct is not Send. It's some future created by some other code that touches it that is not Send, and that error is pointing to the reason.

But I still can't see how moving this expression out is changing it. I thought it's something obviously, but now I'll try to make a repro case.

steffahn · February 7, 2023, 3:20am

Modified to make it work: Rust Playground

quinedot · February 7, 2023, 3:24am

I think:

github.com/rust-lang/rust

async/await: awaiting inside a match block captures borrow too eagerly

opened 07:54PM - 20 Dec 18 UTC

seanmonstar

A-lifetimes T-compiler A-mir A-async-await AsyncAwait-Triaged

If you use `await!(some_fut)` inside an arm of a `match X`, the generated future… eagerly borrows the value of `X`, if it not needed. This may not usually be noticeable, but the issue compounds when the type `X` contains a trait object, and the future you wish to return is `impl Future + Send`. This causes a misleading error message that "`dyn Trait + Send` cannot be shared between threads", which is required to for `&X: Send`. ## Example Here's a simple struct with a trait object: ```rust struct Client(Box<Any + Send>); ``` Consider a function like this: ```rust impl Client { fn status(&self) -> u16 { 200 } } ``` You could consider using a match to determine what kind of future to await (or what arguments to pass): ```rust async fn get() { } pub fn wat() -> impl Future + Send { let client = Client(Box::new(true)); async move { match client.status() { 200 => { let _x = await!(get()); }, _ => (), } } } ``` If the `await` is moved out of the match block, all is well: ```rust pub fn ok() -> impl Future + Send { let client = Client(Box::new(true)); async move { if client.status() == 200 { let _x = await!(get()); } } } ``` The `wat` function causes this compilation error: ``` error[E0277]: `(dyn std::any::Any + std::marker::Send + 'static)` cannot be shared between threads safely --> src/main.rs:21:17 | 21 | pub fn wat() -> impl Future + Send { | ^^^^^^^^^^^^^^^^^^ `(dyn std::any::Any + std::marker::Send + 'static)` cannot be shared between threads safely | = help: the trait `std::marker::Sync` is not implemented for `(dyn std::any::Any + std::marker::Send + 'static)` = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(dyn std::any::Any + std::marker::Send + 'static)>` = note: required because it appears within the type `std::boxed::Box<(dyn std::any::Any + std::marker::Send + 'static)>` = note: required because it appears within the type `Client` = note: required because of the requirements on the impl of `for<'r> std::marker::Send` for `&Client` = note: required because it appears within the type `for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}` = note: required because it appears within the type `[static generator@src/main.rs:23:16: 30:6 client:Client for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}]` = note: required because it appears within the type `std::future::GenFuture<[static generator@src/main.rs:23:16: 30:6 client:Client for<'r> {Client, &'r Client, u16, impl std::future::Future, ()}]>` = note: required because it appears within the type `impl std::future::Future` = note: the return type of a function must have a statically known size ``` Playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=2a9dbea32d31457d50d40b99c52ee214 (updated to latest syntax -Niko)

vague · February 7, 2023, 3:42am

Yes, OP's error msg looks very close to what the comment raised in that issue.
An example from that issue: Rust Playground

gabhijit · February 7, 2023, 7:15am

Sorry, I misread part of your question - It seems the solution above, you have already implemented.

The problem you mention, may be one of that of a design issue highlighted by the borrow checker (too much of shared state). See if it's possible to break the structure into two parts - a) Part that is Send and will be used across await points and part b) that is not Send and can be used only on one side of the await point.

system · May 8, 2023, 7:16am

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.

Topic		Replies	Views
Error complains about missing Sync, but Send is what's missing	2	533	March 16, 2023
Send trait and async/await help	5	749	September 4, 2020
Has type `std::sync::MutexGuard<'_, UpdateProgress>` which is not `Send` help	18	793	April 20, 2023
Future returned by run_consumer() is not Send help	5	247	May 30, 2023
Question about Send and futures help	4	252	October 14, 2023

I don't understand this lifetime + async quirk

Related Topics