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

Async/Await: Confusing error message when using non-Send type in async fn #65667

Closed
dbrgn opened this issue Oct 21, 2019 · 9 comments · Fixed by #65345
Closed

Async/Await: Confusing error message when using non-Send type in async fn #65667

dbrgn opened this issue Oct 21, 2019 · 9 comments · Fixed by #65345
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dbrgn
Copy link
Contributor

dbrgn commented Oct 21, 2019

With this Cargo.toml:

[package]
name = "errmsg"
version = "0.1.0"
edition = "2018"

[dependencies]
futures-preview = { version = "0.3.0-alpha.19", features = ["async-await"] }
tokio = "0.2.0-alpha.6"

...and this main.rs:

use std::rc::Rc;
use std::time::Duration;

use tokio::timer::delay;

async fn wait() {
    let rc = Rc::new(0);
    println!("Before sleep");
    delay(tokio::clock::now() + Duration::from_millis(500)).await;
    println!("After sleep");
}

#[tokio::main]
async fn main() -> Result<(), std::io::Error> {
    println!("Start");
    tokio::spawn(wait());
    println!("End");
    Ok(())
}

...I get this error message:

   Compiling errmsg v0.1.0 (/tmp/errmsg)
error[E0277]: `std::rc::Rc<i32>` cannot be sent between threads safely
   --> src/main.rs:16:5
    |
16  |     tokio::spawn(wait());
    |     ^^^^^^^^^^^^ `std::rc::Rc<i32>` cannot be sent between threads safely
    |
   ::: /home/danilo/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.2.0-alpha.6/src/executor.rs:100:40
    |
100 |     F: Future<Output = ()> + 'static + Send,
    |                                        ---- required by this bound in `tokio::executor::spawn`
    |
    = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<i32>`
    = note: required because it appears within the type `{std::rc::Rc<i32>, fn(std::time::Instant) -> tokio_timer::delay::Delay {tokio_timer::delay}, fn() -> std::time::Instant {tokio_timer::clock::now}, std::time::Instant, fn(u64) -> std::time::Duration {std::time::Duration::from_millis}, u64, std::time::Duration, tokio_timer::delay::Delay, ()}`
    = note: required because it appears within the type `[static generator@src/main.rs:6:17: 11:2 {std::rc::Rc<i32>, fn(std::time::Instant) -> tokio_timer::delay::Delay {tokio_timer::delay}, fn() -> std::time::Instant {tokio_timer::clock::now}, std::time::Instant, fn(u64) -> std::time::Duration {std::time::Duration::from_millis}, u64, std::time::Duration, tokio_timer::delay::Delay, ()}]`
    = note: required because it appears within the type `std::future::GenFuture<[static generator@src/main.rs:6:17: 11:2 {std::rc::Rc<i32>, fn(std::time::Instant) -> tokio_timer::delay::Delay {tokio_timer::delay}, fn() -> std::time::Instant {tokio_timer::clock::now}, std::time::Instant, fn(u64) -> std::time::Duration {std::time::Duration::from_millis}, u64, std::time::Duration, tokio_timer::delay::Delay, ()}]>`
    = note: required because it appears within the type `impl std::future::Future`
    = note: required because it appears within the type `impl std::future::Future`

If I comment out the unused rc assignment, then everything works fine.

If comment out the delay(..).await statement, it works too!

If I put the rc assignment in a block, it compiles as well (because the rc is dropped by the time await is called).

From what I know about the implementation of async-await, I think this is because a state machine is created that keeps the local variables across multiple await-points, right? In any case, the error message is quite confusing, especially when the await call and the non-Send type are not related at all.

It seems that we're back at a pre-NLL situation with the borrow checker in async functions.

In summary, two issues:

  • The error message is quite confusing and not all too helpful, expecially if the user doesn't know anything about the internal implementation of async-await
  • The rc could be dropped before the await call, which would resolve this issue
@estebank estebank added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. F-async_await labels Oct 21, 2019
@nikomatsakis
Copy link
Contributor

Can you clarify @dbrgn whether you are running this on nightly or on beta? We've been doing work on exactly this error message, but it may not be triggering in your particular case.

@nikomatsakis nikomatsakis added AsyncAwait-OnDeck AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Oct 22, 2019
@davidtwco
Copy link
Member

Assigning to myself to check if #65345 addresses this.

@davidtwco davidtwco self-assigned this Oct 22, 2019
@dbrgn
Copy link
Contributor Author

dbrgn commented Oct 22, 2019

@nikomatsakis Sorry for forgetting to mention this. I was on 1.39.0-beta.6.

@davidtwco
Copy link
Member

I can confirm that with #65345 that the error message is improved in this case:

error: future cannot be sent between threads safely
   --> src/main.rs:16:5
    |
16  |     tokio::spawn(wait());
    |     ^^^^^^^^^^^^ future returned by `wait` is not `Send`
    |
   ::: /home/david/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.2.0-alpha.6/src/executor.rs:100:40
    |
100 |     F: Future<Output = ()> + 'static + Send,
    |                                        ---- required by this bound in `tokio::executor::spawn`
    |
    = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<i32>`
note: future is not `Send` as this value is used across an await
   --> src/main.rs:9:5
    |
7   |     let rc = Rc::new(0);
    |         -- has type `std::rc::Rc<i32>`
8   |     println!("Before sleep");
9   |     delay(tokio::clock::now() + Duration::from_millis(500)).await;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `rc` maybe used later
10  |     println!("After sleep");
11  | }
    | - `rc` is later dropped here

error: aborting due to previous error

@dbrgn
Copy link
Contributor Author

dbrgn commented Oct 24, 2019

@davidtwco thanks for checking! This is indeed much better.

Now the question is whether the rc could automatically be dropped before the await call if it's not being used afterwards... But that should probably be handled in a separate issue.

@davidtwco
Copy link
Member

Now the question is whether the rc could automatically be dropped before the await call if it's not being used afterwards... But that should probably be handled in a separate issue.

There was discussion about that in Zulip which might be of interest to you.

I wonder if there is something we could do in this diagnostic when the destructor is the reason that the type lives across the await point to help explain this.

@dbrgn

This comment has been minimized.

@davidtwco

This comment has been minimized.

@nikomatsakis
Copy link
Contributor

@davidtwco I definitely think that destructors are worth trying to "special case" -- we do some similar things in the NLL logic, as I recall.

@bors bors closed this as completed in 27d6f55 Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants