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

Consider async blocks on lifetime errors #64382

Closed
estebank opened this issue Sep 11, 2019 · 2 comments · Fixed by #65166
Closed

Consider async blocks on lifetime errors #64382

estebank opened this issue Sep 11, 2019 · 2 comments · Fixed by #65166
Assignees
Labels
A-async-await A-borrow-checker A-diagnostics AsyncAwait-Polish AsyncAwait-Triaged T-compiler

Comments

@estebank
Copy link
Contributor

@estebank estebank commented Sep 11, 2019

The following incorrect code

        // For QoS, this will only accept one message and output that
        // receive all inbound messages

        // TODO: this should match edns settings
        loop {
            let msg = if let Some(receiving) = receiving {
                // TODO: should we drop this packet if it's not from the same src as dest?
                let msg = ready!(receiving.as_mut().poll(cx))?;

                Some(Poll::Ready(Some(Ok(msg))))
            } else {
                None
            };
 
            *receiving = None;

            if let Some(msg) = msg {
                return msg;
            }

            let socket = Arc::clone(socket);
            let mut buf = [0u8; 2048];
            let receive_future = async {
                let socket = socket;

                let mut socket = socket.lock().await;
                let (len, src) = socket.recv_from(&mut buf).await?;
                
                Ok(SerialMessage::new(
                    buf.iter().take(len).cloned().collect(),
                    src,
                ))
            };

            *receiving = Some(Box::pin(receive_future));
        }

emits the following error

error[E0597]: `buf` does not live long enough
   --> crates/proto/src/udp/udp_stream.rs:204:56
    |
183 |               let msg = if let Some(receiving) = receiving {
    |                                                  --------- borrow later used here
...
200 |               let receive_future = async {
    |  ________________________________________-
201 | |                 let socket = socket;
202 | |
203 | |                 let mut socket = socket.lock().await;
204 | |                 let (len, src) = socket.recv_from(&mut buf).await?;
    | |                                                        ^^^ borrowed value does not live long enough
...   |
209 | |                 ))
210 | |             };
    | |_____________- value captured here by generator
...
213 |           }
    |           - `buf` dropped here while still borrowed

The correct code needs the buf to be part of the async block because it is a stack allocation that needs to be in the generator:

        loop {
            let msg = if let Some(receiving) = receiving {
                // TODO: should we drop this packet if it's not from the same src as dest?
                let msg = ready!(receiving.as_mut().poll(cx))?;

                Some(Poll::Ready(Some(Ok(msg))))
            } else {
                None
            };
 
            *receiving = None;

            if let Some(msg) = msg {
                return msg;
            }

            let socket = Arc::clone(socket);
            let receive_future = async {
                let socket = socket;
                let mut buf = [0u8; 2048];

                let mut socket = socket.lock().await;
                let (len, src) = socket.recv_from(&mut buf).await?;
                
                Ok(SerialMessage::new(
                    buf.iter().take(len).cloned().collect(),
                    src,
                ))
            };

            *receiving = Some(Box::pin(receive_future));
        }

Ideally, the error message would mention buf:

error[E0597]: `buf` does not live long enough
   --> crates/proto/src/udp/udp_stream.rs:204:56
    |
183 |               let msg = if let Some(receiving) = receiving {
    |                                                  --------- borrow later used here
...
199 |               let mut buf = [0u8; 2048];
    |                       --- consider moving this stack allocation inside the `async` block
200 |               let receive_future = async {
    |  ________________________________________-
201 | |                 let socket = socket;
202 | |
203 | |                 let mut socket = socket.lock().await;
204 | |                 let (len, src) = socket.recv_from(&mut buf).await?;
    | |                                                        ^^^ borrowed value does not live long enough
...   |
209 | |                 ))
210 | |             };
    | |_____________- value captured here by `async` block
...
213 |           }
    |           - `buf` dropped here while still borrowed

thanks to @bluejekyll for bringing this up!

CC @nikomatsakis

@estebank estebank added A-diagnostics A-borrow-checker T-compiler A-async-await AsyncAwait-Unclear labels Sep 11, 2019
@cramertj
Copy link
Member

@cramertj cramertj commented Sep 24, 2019

We could also consider suggesting an async move block, similar to the hints we provide for closures.

@cramertj cramertj added AsyncAwait-Triaged and removed AsyncAwait-Unclear labels Sep 24, 2019
@nikomatsakis nikomatsakis added AsyncAwait-OnDeck AsyncAwait-Polish and removed AsyncAwait-OnDeck AsyncAwait-Triaged labels Oct 1, 2019
@nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Oct 1, 2019

Marking this as a focus item.

@rustbot assign @csmoe

@nikomatsakis nikomatsakis added the AsyncAwait-Triaged label Oct 8, 2019
Centril added a commit to Centril/rust that referenced this issue Oct 9, 2019
Suggest to add `move` keyword for generator capture

 Closes rust-lang#64382
r? @estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await A-borrow-checker A-diagnostics AsyncAwait-Polish AsyncAwait-Triaged T-compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants