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

confusing "lifetime mismatch" error when spawning task #86487

Open
yoshuawuyts opened this issue Jun 20, 2021 · 2 comments
Open

confusing "lifetime mismatch" error when spawning task #86487

yoshuawuyts opened this issue Jun 20, 2021 · 2 comments
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. C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Jun 20, 2021

suggested labels: A-diagnostics, A-async-await, D-confusing, requires-nightly

Introduction

Hey diagnostics team. I'm currently helping a pal debug some code, and we're experiencing a particularly gnarly error. We haven't been able to figure out what's going wrong, so we can't provide a repro either. So I'm opening this issue so folks can come along on the journey we're on, and maybe we can figure out both what the root error is -- and how to improve the error messages -- together!

Setting the scene

The current project lives on the spawn branch of the peermap-ingest project. To get started do:

$ git clone https://github.com/substack/peermaps-ingest
$ git checkout origin spawn   # latest commit at time of writing is: ebb442c8716a0c6fa5d511756fcde940726f6be4
$ rustup override set nightly # rust version used at the time of writing is 1.54.0-nightly MSVC
$ cargo check

The following code is where our error points us to. We're pushing async-std task::JoinHandle into a Vec so we can await all handles together later on. Each task processes a command, which interacts with a backing store asynchronously in the background. If an error occurs, we log it. The async move {} block's generated future returns a ().

for _ in 0..nthreads {
    let this = self.clone();
    let r = receiver.clone();
    work.push(async_std::task::spawn(async move {
        loop {
            if r.is_closed() {
                break;
            }
            if let Ok((key, value)) = r.recv().await {
                let res: Result<(), Error> = match decode(&key.data, &value) {
                    Err(e) => Err(e.into()),
                    Ok(Decoded::Node(node)) => this.create_node(&node).await,
                    Ok(Decoded::Way(way)) => this.create_way(&way).await,
                    Ok(Decoded::Relation(relation)) => this.create_relation(&relation).await,
                };
                if let Some(f) = this.reporter.lock().await.as_mut() {
                    match res {
                        Err(e) => f(Phase::Process(), Err(e.into())),
                        Ok(r) => f(Phase::Process(), Ok(r)),
                    }
                }
            }
        }
    }));
}

Running the code generates the following output:

PS C:\Users\yoshu\Code\peermaps-ingest> cargo c
    Checking peermaps-ingest v1.0.2 (C:\Users\yoshu\Code\peermaps-ingest)
error[E0308]: mismatched types
   --> src\lib.rs:148:17
    |
148 |       work.push(async_std::task::spawn(async move {
    |                 ^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
    |
    = note: expected type `Result<_, Box<dyn std::error::Error + std::marker::Send + std::marker::Sync>>`
               found enum `Result<_, Box<dyn std::error::Error + std::marker::Send + std::marker::Sync>>`
note: the lifetime requirement is introduced here
   --> C:\Users\yoshu\scoop\persist\rustup\.cargo\registry\src\github.com-1ecc6299db9ec823\async-std-1.9.0\src\task\spawn.rs:28:29
    |
28  |     F: Future<Output = T> + Send + 'static,
    |                             ^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `peermaps-ingest`

What are we looking at?

The main error describes a: "lifetime mismatch". It then points to a call site, highlights the type signatures, and finally points to where the requirement was defined. However this has some issues:

  • the types in note: expected type do not carry any explicit lifetimes, and look otherwise identical to each other
  • note: the lifetime requirement is introduced here points to the Send param in async_std::task::spawn. That's not a lifetime, which makes it confusing.
  • we have to assume that the mention of "lifetime" here refers to the 'static lifetime in async_std::task::spawn, but we cannot know for sure.
  • in fact, given Send is highlighted in the second output makes it unclear whether the issue actually is that we are trying to pass a !Send item to a function which required the item is Send.
  • the mention of Result<_, Box<dyn Error + ...>> is odd, since the return type of the Future is (). What error is being referred to here? It's unclear!

What have we tried so far?

  • I've added + 'static notations to every Error definition in the code (and dependencies) to no change
  • I've replaced the anonymous async move {} block with an async fn to no change
  • I've manually gone through the function calls and into dependencies, commenting out code to find the root cause -- but I couldn't find any return types which were !Send or non-'static (after having done the error type fix as well)

What could be done to help solve this?

  • It's unclear whether it's an issue with Send or an issue with a lifetime (if so: which lifetime?). Clearing that up would be a first step.
  • The two types in hint: expected type should not be identical. If a lifetime is different, it should probably show up.
  • The link between async_std::task::spawn in the source, and the error in the hint: expected type is unclear. It'd be nice to know which Error type is being referred to -- even if it's only shown with --verbose output. Needing to manually step through code takes a lot of time, where supposedly the compiler already knows this information (even if not displayed in the output).

Conclusion

I hope this is enough information for someone to take a look at this. I think we're hitting a particularly gnarly combination of diagnostics failures here, which when put together make it hard to debug. Thanks heaps!

cc/ @substack

@yoshuawuyts yoshuawuyts added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 20, 2021
@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Jun 20, 2021

I agree that the diagnostics here are completely unable to point to the right direction.


The trick is to comment out .await lines until it passes, and then try to pin point why the .await is problematic:

  • is a non-Send captured var crossing the .await?

  • is there an intermediate future which is not impl Send? (this is actually a sub instance of the previous bullet, but in practice is debugged / treated quite differently).


By applying this, we find out that, for instance, async fn create_node is not yielding an impl 'static + Send + Future.

Then, rince and repeat. For instance, I have tweaked the body of that function until reaching:

  /// First, unsugar the `async fn` into an `fn … -> impl Future…`:
  fn create_node<'slf, 'node, 'fut>(self: &'slf Self, node: &'node DecodedNode)
    -> impl 'fut + Future<Output = Result<(),Error>> + Send
  where
    &'slf Self : 'fut, // 'slf : 'fut
    &'node DecodedNode : 'fut, // 'node : 'fut
  {
    async move { let _captures_args = (&self, &node);
      if let Some((point,encoded)) = self.encode_node(node)? {
        let mut estore = self.estore.lock().await;
        let () = async {}.await; // to make `estore` cross an await point

        // with the following trick, we get a value of the right type (`Result…`),
        // but without the intermediate future polluting the state machine.
        let res = /* estore.create(point, encoded.into()).await */ {
          let mut it = None.unwrap();
          if false { // type-inference arm.
            let _ = async {
              it = estore.create(point, encoded.into()).await;
            };
            loop {}
          }
          it
        };
        // All these checks pass.
        let _: &(dyn Send + 'static) = &res;
        if let Err(err) = res {
          let err = Error::from(err);
          let _: &(dyn Send + 'static) = &err;
          return Err(err);
        }
      }
      Ok(())
    }
  }

And when not having the future of estore.create(point, encoded.into()) polluting the state machine, we do meet the impl 'fut + Send requirements, so the issue is with the async fn create on estore, … so on and so forth (I have stopped once I noticed that the futures from ::eyros where not Send).


Note that for the type inference tricks and so on, you can use the following macro to make it more readable and easy to apply:

macro_rules! dummy_of_typeof {( $e:expr $(,)? ) => ({
    let mut it = ::core::option::Option::None.expect("dummy_of_typeof!");
    if false { // help type inference
        let _ = async { it = $e; };
        loop {}
    }
    it
})}

and then do:

  fn create_node<'fut>(&'fut self, node: &'fut DecodedNode)
    -> impl 'fut + Future<Output = Result<(),Error>> + Send
  {
    async move {
      let _captured = (self, node);
      if let Some((point,encoded)) = self.encode_node(node)? {
        let mut estore = self.estore.lock().await;
        let () = async {}.await; // to make `estore` cross an await point
        let res = dummy_of_typeof![ estore.create(point, encoded.into()).await ];
        let _: &dyn Send = &res;
        res?;
      }
    }
  }

which is a bit lighter to write and to use when debugging.

@JohnTitor JohnTitor added A-async-await Area: Async & Await D-confusing Diagnostics: Confusing error or lint that should be reworked. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example requires-nightly This issue requires a nightly compiler in some way. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jun 21, 2021
@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Jun 21, 2021

Addendum: I've found that with #![allow(clippy::all)] #![warn(clippy::future_not_send)] at the root of the lib.rs, running cargo clippy yields outstanding diagnostics, @yoshuawuyts

  • on eyros:

    Screen Shot 2021-06-21 at 20 48 31
    • where we can spot some missing Send bounds on some dyn Streams, for instance.
  • on your crate:

    Screen Shot 2021-06-21 at 21 02 45

Regarding the labels, there is maybe a C-bug here w.r.t. diagnostics, one which is known: adding a Send constraint on a future can trigger a "back-pressuring trait solver / type existential type resolution" which causes these lifetime errors, which thus shadow the expected nice error messages about futures not being Send, which, to be honest, is a pity since nowadays non-Send-futures errors are usually outstandingly good 🙂

@rustbot label: -C-enhancement +C-bug .

@tmandry tmandry added AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jun 25, 2021
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. C-bug Category: This is a bug. D-confusing Diagnostics: Confusing error or lint that should be reworked. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants