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 borrows more around await points than necessary #61211

Closed
sdroege opened this issue May 26, 2019 · 9 comments
Closed

async/await borrows more around await points than necessary #61211

sdroege opened this issue May 26, 2019 · 9 comments
Labels
A-async-await Area: Async & Await A-borrow-checker Area: The borrow checker AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sdroege
Copy link
Contributor

sdroege commented May 26, 2019

    let foo = Mutex::new("123");

    let foo: Box<dyn futures::future::Future<Output = ()> + Send> = Box::new(async move {
        // In a separate block works
        // {
        let bar = foo.lock().unwrap();
        drop(bar);
        // }
        futures::future::lazy(|_| ()).await;
    });

Fails with

error[E0277]: `std::sync::MutexGuard<'_, &str>` cannot be sent between threads safely

When putting the lock into a separate block it works fine, when not awaiting or locking after the last await point it also works fine.

It seems that dropped values at await points are still considered in scope and borrowed.

@Centril Centril added A-async-await Area: Async & Await A-borrow-checker Area: The borrow checker labels May 26, 2019
@pnkfelix
Copy link
Member

This ... might be a deliberate choice?

In particular, there has been some discussion circling around whether the behavior in cases like this should be based on lexical scopes (as I believe is currently the case) or a liveness analysis (which what I think @sdroege is implicitly asking for here)?

But I'm not an expert in what is the general expectation when it come to async-await semantics. cc @cramertj and @withoutboats

@withoutboats
Copy link
Contributor

Maybe I'm confused, but I don't think this is a deliberate choice and I think its connected #52924.

What we've decided is that we don't rearrange destructors in any surprising way, but once something has been dropped we shouldn't need to hold it in the state any longer. I don't think fixing this is a blocker (since it should be strictly backwards compatible for these generators to gain a Send impl), but I think this is a bug.

@cramertj
Copy link
Member

cc @tmandry

@sdroege
Copy link
Contributor Author

sdroege commented May 28, 2019

I don't think fixing this is a blocker [...] but I think this is a bug.

I agree, and it's also easy enough to workaround and you'll notice at compile time. It's mostly an inconvenience.

@tmandry
Copy link
Member

tmandry commented May 29, 2019

Typecheck has its own "view" of what locals might get incorporated into the generator, and that would be what's generating this error.

That's separate from the layout code (except that the layout set of saved locals must be a subset of the typecheck set). So I would say this isn't really part of #52924.

EDIT: The typeck set is the one in GeneratorWitness.

@nikomatsakis
Copy link
Contributor

This is I think a dup of #57017

@nikomatsakis
Copy link
Contributor

Based on that, I'm going to mark as deferred -- probably we should move the example into #57017 and close this issue though

@nikomatsakis nikomatsakis added AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 4, 2019
@sdroege
Copy link
Contributor Author

sdroege commented Jun 5, 2019

Based on that, I'm going to mark as deferred -- probably we should move the example into #57017 and close this issue though

Added it in a comment there. Thanks!

@sdroege sdroege closed this as completed Jun 5, 2019
@Nemo157
Copy link
Member

Nemo157 commented Jun 12, 2019

#57478 is another more specific dup.

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-borrow-checker Area: The borrow checker AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. 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

8 participants