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

[Stabilization] async/await MVP #62149

Closed
2 of 4 tasks
withoutboats opened this issue Jun 26, 2019 · 58 comments · Fixed by #63209
Closed
2 of 4 tasks

[Stabilization] async/await MVP #62149

withoutboats opened this issue Jun 26, 2019 · 58 comments · Fixed by #63209
Labels
A-async-await Area: Async & Await AsyncAwait-Polish Async-await issues that are part of the "polish" area disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@withoutboats
Copy link
Contributor

withoutboats commented Jun 26, 2019

Stabilization target: 1.38.0 (beta cut 2019-08-15)

Executive Summary

This is a proposal to stabilize a minimum viable async/await feature, which includes:

  • async annotations on functions and blocks, causing them to be delayed in evaluation and instead evaluate to a future.
  • An await operator, valid only within an async context, which takes a future as an argument and causes the outer future it is within to yield control until the future being awaited has completed.

Related previous discussions

RFCs:

Tracking issues:

Stabilizations:

Major decisions reached

  • The future that an async expression evaluates to is constructed from its initial state, running none of the body code before yielding.
  • The syntax for async functions uses the "inner" return type (the type that matches the internal return expression) rather than the "outer" return type (the future type that a call to the function evaluates to)
  • The syntax for the await operator is the "postfix dot syntax," expression.await, as opposed to the more common await expression or another alternative syntax.

Implementation work blocking stabilization

Future work

  • Async/await in no-std contexts: async and await currently rely on TLS to work. This is an implementation issue that is not a part of the design, and though it is not blocking stabilization it is intended to be resolved eventually.
  • Higher order async functions: async as a modifier for closure literals is not stabilized here. More design work is needed regarding capture and abstraction over async closures with lifetimes.
  • Async trait methods: This involves significant design and implementation work, but is a highly desirable feature.
  • Stream processing: The pair to the Future trait in the futures library is the Stream trait, an asynchronous iterator. Integrating support to manipulating streams into std and the language is a desirable long term feature.
  • Optimizing generator representations: More work can be done to optimize the representation of generators to make them more perfectly sized. We have ensured that this is strictly an optimization issue and is not semantically significant.

Background

Handling non-blocking IO is very important to developing high performance network services, a target use case for Rust with significant interest from production users. For this reason, a solution for making it ergonomic and feasible to write services using non-blocking IO has long been a goal of Rust. The async/await feature is the culmination of that effort.

Prior to 1.0, Rust had a greenthreading system, in which Rust provided an alternative, language-level threading primitive built on top of nonblocking IO. However, this system caused several problems: most importantly introducing a language runtime that impacted the performance even of programs that did not use it, adding significantly to the overhead of FFI, and having several major unresolved design problems to do with the implementation of greenthread stacks.

After the removal of greenthreads, members of the Rust project began working on an alternative solution based on the futures abstraction. Sometimes also called promises, futures had been very successful in other languages as a library-based abstraction for nonblocking IO, and it was known that in the long term they mapped well to an async/await syntax which could make them only minorly less convenient than a completely invisible greenthreading system.

The major breakthrough in the development of the Future abstraction was the introduction of a poll-based model for futures. While other languages use a callback based model, in which the future itself is responsible for scheduling the callback to be run when it is complete, Rust uses a poll based model, in which an executor is responsible for polling the future to completion, and the future merely informing the executor that it is ready to make further progress using the Waker abstraction. This model worked well for several reasons:

  • It enabled rustc to compile futures to state machines which had the most minimal memory overhead, both in terms of size and indirection. This has significant performance benefits over the callback based approach.
  • It allows components like the executor and reactor to exist as library APIs, rather than a part of the language runtime. This avoids introducing global costs that impact users who are not using this feature, and allows users to replace individual components of their runtime system easily, rather than requiring us to make a blackbox decision for them at the language level.
  • It makes all concurrency primitives libraries as well, rather than baking concurrency into the language through the semantics of the async and await operators. This makes concurrency clearer and more visibile through the source text, which must use an identifiable concurrency primitive to introduce concurrency.
  • It allows for cancellation without overhead, by allowing executing futures to be dropped before they are completed. Making all futures cancellable for free has performance and code clarity benefits for executors and concurrency primitives.

(The last two points have also been identified as a source of confusion for users coming from other languages in which they are not true, and bringing expectations from those languages with them. However, these properties are both unavoidable properties of the poll-based model which has other clear advantages and are, in our opinion, beneficial properties once users understand them.)

However, the poll-based model suffered from serious ergonomic issues when it interacted with references; essentially, references across yield points introduced unresolvable compilation errors, even though they should be safe. This resulted in complex, noisy code full of arcs, mutexes, and move closures, none of which was strictly necessary. Even setting this problem aside, without language level primitive, futures suffered from forcing users into a style of writing highly nested callbacks.

For this reason, we pursued async/await syntactic sugar with support for normal use of references across yield points. After introducing the Pin abstraction which made references across yield points safe to support, we have developed a native async/await syntax which compiles functions into our poll-based futures, allowing users to get the performance advantages of asynchronous IO with futures while writing code which is very similar to standard imperative code. That final feature is the subject of this stabilization report.

async/await feature description

The async modifier

The keyword async can be applied in two places:

  • Before a block expression.
  • Before a free function or an associated function in an inherent impl.

(Other locations for async functions - closure literals and trait methods, for example, will be developed further and stabilized in the future.)

The async modifier adjusts the item it modifies by "turning it into a future." In the case of a block, the block is evaluated to a future of its result, rather than its result. In the case of a function, calls to that function return a future of its return value, rather than its return value. Code inside an item modified by an async modifier is referred to as being in an async context.

The async modifier performs this modification by causing the item to instead be evaluated as a pure constructor of a future, taking arguments and captures as fields of the future. Each await point is treated as a separate variant of this state machine, and the future's "poll" method advances the future through these states based on a transformation of the code the user wrote, until eventually it reaches its final state.

The async move modifier

Similar to closures, async blocks can capture variables in the surrounding scope into the state of the future. Like closures, these variables are by default captured by reference. However, they can instead be captured by value, using the move modifier (just like closures). async comes before move, making these blocks async move { } blocks.

The await operator

Within an async context, a new expression can be formed by combining an expression with the await operator, using this syntax:

expression.await

The await operator can only be used inside an async context, and the type of the expression it is applied to must implement the Future trait. The await expression evaluates to the output value of the future it is applied to.

The await operator yields control of the future that the async context evaluates to until the future it is applied to has completed. This operation of yielding control cannot be written in the surface syntax, but if it could (using the syntax YIELD_CONTROL! in this example) the desugaring of await would look roughly like this:

loop {
    match $future.poll(&waker) {
        Poll::Ready(value)  => break value,
        Poll::Pending       => YIELD_CONTROL!,
    }
}

This allows you to wait for futures to finish evaluating in an async context, forwarding the yielding of control through Poll::Pending outward to the outermost async context, ultimately to the executor onto which the future has been spawned.

Major decision points

Yielding immediately

Our async functions and blocks "yield immediately" - constructing them is a pure function that puts them in an initial state prior to executing code in the body of the async context. None of the body code gets executed until you begin polling that future.

This is different from many other languages, in which calls to an async function trigger work to begin immediately. In these other languages, async is an inherently concurrent construct: when you call an async function, it triggers another task to begin executing concurrent with your current task. In Rust, however, futures are not inherently executed in a concurrent fashion.

We could have async items execute up to the first await point when they are constructed, instead of making them pure. However, we decided this was more confusing: whether code is executed during constructing the future or polling it would depend on the placement of the first await in the body. It is simpler to reason about for all code to be executed during polling, and never during construction.

Reference:

Return type syntax

The syntax of our async functions uses the "inner" return type, rather than the "outer" return type. That is, they say that they return the type that they eventually evaluate to, rather than saying that they return a future of that type.

On one level, this is a decision about what kind of clarity is preferred: because the signature also includes the async annotation, the fact that they return a future is made explicit in the signature. However, it can be helpful for users to see that the function returns a future without having to notice the async keyword as well. But this also feels like boilerplate, since the information is conveyed also by the async keyword.

What really tipped the scales for us was the issue of lifetime elision. The "outer" return type of any async function is impl Future<Output = T>, where T is the inner return type. However, that future also captures the lifetimes of any input arguments in itself: this is the opposite of the default for impl Trait, which is not assumed to capture any input lifetimes unless you specify them. In other words, using the outer return type would mean that async functions never benefited from lifetime elision (unless we did something even more unusual like having lifetime elision rules work differently for async functions and other functions).

We decided that given how verbose and frankly confusing the outer return type would actually be to write, it was not worth the extra signalling that this returns a future to require users to write it.

Destructor ordering

The ordering of destructors in async contexts is the same as in non-async contexts. The exact rules are a bit complicated and out of scope here, but in general, values are destroyed when they go out of scope. This means, though, that they continue to exist for some time after they are used until they get cleaned up. If that time includes await statements, those items need to be preserved in the state of the future so their destructors can be run at the appropriate time.

We could, as an optimization to the size of future states, instead re-order destructors to be earlier in some or all contexts (for example, unused function arguments could be dropped immediately, instead of being stored in the state of the future). However, we decided not to do this. The order of destructors can be a thorny and confusing issue for users, and is sometimes very significant for program semantics. We've chosen to forego this optimization in favor of guaranteeing a destructor ordering that is as straightforward as possible - the same destructor ordering if all of the async and await keywords were removed.

(Someday, we may be interested in pursuing ways of marking destructors as pure and re-orderable. That is future design work that has implications unrelated to async/await as well.)

Reference:

Await operator syntax

One major deviation from other languages' async/await features is the syntax of our await operator. This has been the subject of an enormous amount of discussion, more than any other decision we've made in the design of Rust.

Since 2015, Rust has had a postfix ? operator for ergonomic error handling. Since long before 1.0, Rust has also had a postfix . operator for field access and method calls. Because the core use case for futures is to perform some sort of IO, the vast majority of futures evaluate to a Result with some
sort of error. This means that in practice, nearly every await operation is sequenced with either a ? or a method call after it. Given the standard precedence for prefix and postfix operators, this would have caused nearly every await operator to be written (await future)?, which we regarded as highly unergonomic.

We decided therefore to use a postfix syntax, which composes very well with the ? and . operators. After considering many different syntactic options, we chose to use the . operator followed by the await keyword.

Reference:

Supporting both single and multithreaded executors

Rust is designed to make writing concurrent and parallel programs easier without imposing costs on people writing programs that run on a single thread. It's important to be able to run async functions both on singlethreaded executors and multithreaded executors. The key difference between these two use cases is that multithreaded executors will bound the futures they can spawn by Send, and singlethreaded executors will not.

Similar to the existing behavior of impl Trait syntax, async functions "leak" the auto traits of the future they return. That is, in addition to observing that the outer return type is a future, the caller can also observe if that type is Send or Sync, based on an examination of its body. This means that when the return type of an async fn is scheduled onto a multithreaded executor, it can check whether or not this is safe. However, the type is not required to be Send, and so users on singlethreaded executors can take advantage of more performant single-threaded primitives.

There was some concern that this would not work well when async functions were expanded into methods, but after some discussion it was determined that the situation would not be significantly different.

Reference:

Known stabilization blockers

State size

Issue: #52924

The way the async transformation to a state machine is currently implemented not at all optimal, causing the state to become much larger than necessary. It's possible, because the state size actually grows superlinearly, to trigger stack overflows on the real stack as the state size grows larger than the size of a normal system thread. Improving this codegen so that the size is more reasonable, at least not bad enough to cause stack overflows in normal use, is a blocking bug fix.

Multiple lifetimes in async functions

Issue: #56238

async functions should be able to have multiple lifetimes in their signature, all of which are "captured" in the future the function is evaluated to when it is called. However, the current lowering to impl Future inside the compiler does not support multiple input lifetimes; a deeper refactor is needed to make
this work. Because users are very likely to write functions with multiple (probably all elided) input lifetimes, this is a blocking bug fix.

Other blocking issues:

Label

Future work

All of these are known and very high priority extensions to the MVP that we intend to pick up work on as soon as we have shipped the initial version of async/await.

Async closures

In the initial RFC, we also supported the async modifier as a modifier on closure literals, creating anonymous async functions. However, experience using this feature has shown that there are still a number of design questions to resolve before we feel comfortable stabilizing this use case:

  1. The nature of variable capture becomes more complicated in async closures and make require some syntactic support.
  2. Abstracting over async functions with input lifetimes is currently not possible and may require some additional language or library support.

No-STD support

The current implementation of the await operator requires TLS to pass the waker downward as it polls the inner future. This is essentially a "hack" to make the syntax work on systems with TLS as soon as possible. In the long term, we have no intention of committing to this usage of TLS, and would prefer to pass the waker as a normal function argument. However, this requires deeper changes to the state machine generation code so that it can handle taking arguments.

Though we are not blocking on implementing this change, we do consider it a high priority as it prevents using async/await on systems without TLS support. This is a pure implementation issue: nothing in the design of the system requires TLS usage.

Async trait methods

We do not currently allow async associated functions or methods in traits; this is the only place in which you can write fn but not async fn. Async methods would very clearly be a powerful abstraction and we want to support them.

An async method would functionally be treated as a method returning an associated type that would implement future; each async method would generate a unique future type for the state machine that that method translates into.

However, because that future would capture all input, any input lifetime or type parameters would need to be captured in that state as well. This is equivalent to a concept called generic associated types, a feature we have long wanted but have not yet properly implemented. Thus, the resolution of async methods is tied to the resolution of generic associated types.

There are also outstanding design issues. For example, are async methods interchangeable with methods returning future types that would have the same signature? Additionally, async methods present additional issues around auto traits, since you may need to require that the future returned by some async method implements an auto trait when you abstract over a trait with an async method.

Once we have even this minimal support, there are other design considerations for future extensions, like the possibility of making async methods "object safe."

Generators and async generators

We have an unstable generator feature using the same coroutine state machine transformation to take functions which yield multiple values and turn them into state machines. The most obvious use case for this feature is to create functions that compile to "iterators," just as async functions compile to
futures. Similarly, we could compose these two features to create async generators - functions that compile to "streams," the async equivalent of iterators. There are really clear use cases for this in network programming, which often involves streams of messages being sent between systems.

Generators have a lot of open design questions because they are a very flexible feature with many possible options. The final design for generators in Rust in terms of syntax and library APIs is still very up in the air and uncertain.

@withoutboats withoutboats added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-async-await Area: Async & Await labels Jun 26, 2019
@withoutboats
Copy link
Contributor Author

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jun 26, 2019

Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 26, 2019
@Centril
Copy link
Contributor

Centril commented Jun 26, 2019

(Just registering the existing blockers in the report above to make sure they don't slip)

@rfcbot concern implementation-work-blocking-stabilization

@vi

This comment has been minimized.

@Centril

This comment has been minimized.

@scottmcm
Copy link
Member

Wow, thank you for the comprehensive summary! I've only been following tangentially, but am completely confident you're on top of everything.

@rfcbot reviewed

@Ekleog
Copy link

Ekleog commented Jun 26, 2019

Could it be possible to explicitly add “Triage AsyncAwait-Unclear issues” to the stabilization blockers (and/or register a concern for that)?

I've got #60414 that I think is important (obviously, it's my bug :p), and would like to at least have it explicitly deferred before stabilization :)

@CAD97
Copy link
Contributor

CAD97 commented Jun 26, 2019

I'd just like to express the community thanks for the effort the Rust teams have put into this feature! There's been a lot of design, discussion, and a few breakdowns in communication, but at least I, and hopefully many others, feel confident that through it all we've found the best solution possible for Rust. 🎉

(That said, I'd like to see a mention of the problems with bridging to completion-based and async-cancellation system APIs in future possibilities. TL;DR they still have to pass around owned buffers. It's a library issue, but one with mentioning.)

@newpavlov

This comment has been minimized.

@cramertj

This comment has been minimized.

@cramertj
Copy link
Member

@Ekleog

Could it be possible to explicitly add “Triage AsyncAwait-Unclear issues” to the stabilization blockers (and/or register a concern for that)?

Yup, that's something we've been doing every week. WRT that specific issue (#60414), I believe it's important and would love to see it fixed, but we haven't yet been able to decide whether or not it should block stabilization, especially since it's already observable in -> impl Trait functions.

@Ekleog
Copy link

Ekleog commented Jun 26, 2019

@cramertj Thank you! I think #60414 's issue is basically “the error can arise really quickly now”, while with -> impl Trait it looks like no one had even noticed it before -- then it's alright if it gets deferred anyway, some issues will have to :) (FWIW it arose in natural code in a function where I return both () at a place and T::Assoc at another, which IIRC made me unable to get it to compile -- haven't checked the code since opening #60414, though, so maybe my recollection is wrong)

@cramertj
Copy link
Member

@Ekleog Yeah that makes sense! I can definitely see why it'd be a pain-- I've created a zulip stream to dive more into that specific issue.

@theduke

This comment has been minimized.

@newpavlov

This comment has been minimized.

@huxi

This comment has been minimized.

@theduke

This comment has been minimized.

@cramertj

This comment has been minimized.

@withoutboats
Copy link
Contributor Author

(That said, I'd like to see a mention of the problems with bridging to completion-based and async-cancellation system APIs in future possibilities. TL;DR they still have to pass around owned buffers. It's a library issue, but one with mentioning.)

I also would like to see a mention of problems with completion-based APIs. (see this internals thread for context) Considering IOCP and introduction of io_uring, which may become The Way for async IO on Linux, I think it's important to have a clear way forward for handling them.

I agree with Taylor that discussing API designs in this problem space would be off topic, but I do want to address one specific aspect of these comments (and this discussion around io_uring in general) that is relevant to async/await stabilization: the problem of timing.

io_uring is an interface that is coming to Linux this year, 2019. The Rust project has been working on the futures abstraction since 2015, four years ago. The fundamental choice to favor a poll based over a completion based API occurred during 2015 and 2016. At RustCamp in 2015, Carl Lerche talked about why he made that choice in mio, the underlying IO abstraction. In this blog post in 2016, Aaron Turon talked about the benefits for creating higher level abstractions. These decisions were made a long time ago and we could not have gotten to the point we are now without them.

Suggestions that we should revisit our underlying futures model are suggestions that we should revert back to the state we were in 3 or 4 years ago, and start over from that point. What kind of abstraction could cover a completion-based IO model without introducing overhead for higher level primitives, like Aaron described? How will we map that model to a syntax that lets users write "normal Rust + minor annotations" the way async/await does? How will we be able to handle integrating that into our memory model, as we've done for these state machines with pin? Trying to provide answers to these questions would be off-topic for this thread; the point is that answering them, and proving the answers correct, is work. What amounts to a solid decade of labor-years between the different contributors so far would have to be redone again.

The goal of Rust is to ship a product that people can use, and that means we have to ship. We can't always be stopping to look into the future at what may become a big deal next year, and restarting our design process to incorporate that. We do the best we can based on the situation we find ourselves in. Obviously it can be frustrating to feel like we barely missed a big thing, but as it stands we don't have a full view either a) of what the best outcome for handling io_uring will be, b) how important io_uring will be in the ecosystem as a whole. We can't revert 4 years of work based on this.

There are already similar, probably even more serious, limitations of Rust in other spaces. I want to highlight one I looked at with Nick Fitzgerald last fall: wasm GC integration. The plan for handling managed objects in wasm is to essentially segment the memory space, so that they exist in a separate address space from unmanaged objects (indeed, someday in many separate address spaces). Rust's memory model is simply not designed to handle separate address spaces, and any unsafe code that deals with heap memory today assumes there is only 1 address space. While we've sketched out both breaking and technically-nonbreaking-but-extremely-disruptive technical solutions, the most likely path forward is to accept that our wasm GC story may not be perfectly optimal, because we are dealing with the limitations of Rust as it exists.

@RalfJung
Copy link
Member

RalfJung commented Jun 29, 2019

An interesting aspect that we are stabilizing here is that we are making self-referential structs available from safe code. What makes this interesting is that in a Pin<&mut SelfReferentialGenerator>, we have a mutable reference (stored as a field in the Pin) pointing to the entire generator state, and we have a pointer inside that state pointing to another piece of the state. That inner pointer aliases with the mutable reference!

The mutable reference, to my knowledge, does not get used to actually access the part of the memory that the pointer-to-another-field points so. (In particular, there is no clone method or so that would read the pointer-to field using any other pointer than the self-referential one.) Still, this is getting way closer to having a mutable reference alias with something than anything else in the core ecosystem, in particular anything else that ships with rustc itself. The "line" we are riding here is getting very thin, and we have to be careful not to lose all these nice optimizations that we want to do based on mutable references.

There's probably little we can do about that at this point, in particular since Pin is already stable, but I feel it is worth pointing out that this will significantly complicate whatever the rules end up being for which aliasing is allowed and which is not. If you thought Stacked Borrows was complicated, prepare for things getting worse.

Cc rust-lang/unsafe-code-guidelines#148

Centril added a commit to Centril/rust that referenced this issue Jun 30, 2019
…etrochenkov

Always parse 'async unsafe fn' + properly ban in 2015

Parse `async unsafe fn` not `unsafe async fn` in implementations. We also take the opportunity to properly ban `async fn` in Rust 2015 when they are inside implementations.

Closes rust-lang#62232.

cc rust-lang#61319, rust-lang#62121, and rust-lang#62149.

r? @petrochenkov
@withoutboats
Copy link
Contributor Author

The mutable reference, to my knowledge, does not get used to actually access the part of the memory that the pointer-to-another-field points so.

People have talked about making all of these coroutine types implement Debug, it sounds like that conversation should also integrate unsafe code guidelines to be sure what its safe to debug print.

@RalfJung
Copy link
Member

RalfJung commented Jul 1, 2019

People have talked about making all of these coroutine types implement Debug, it sounds like that conversation should also integrate unsafe code guidelines to be sure what its safe to debug print.

Indeed. Such a Debug implementation, if it prints the self-referenced fields, would likely prohibit MIR-level reference-based optimizations inside generators.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 13, 2019

The alternative is not viable to implement in the near or medium term (meaning several years). There is no way to create a future which is unsafe to poll in the currently designed Rust language.

FWIW I ran into a similar problem that I mentioned in RFC2585 when it comes to closures inside unsafe fn and the function traits. I didn't expect unsafe async fn to return a Future with a safe poll method, but instead to return an UnsafeFuture with an unsafe poll method. (*) We could then make .await also work on UnsafeFutures when it is used inside of unsafe { } blocks, but not otherwise.

These two future traits would be a huge change with respect to what we have today, and they would probably introduce a lot of composability issues. So the ship for exploring alternatives has probably sailed. Particularly since this would be different to how the Fn traits work today (e.g. we don't have a UnsafeFn trait or similar, and my issue in RFC2585 was that creating a closure inside an unsafe fn returns a closure that impls Fn(), that is, that is safe to call, even though this closure can call unsafe functions.

Creating the "unsafe" Future or closure is not the problem, the problem is calling them without proving that doing so is safe, particularly when their types do not say that this must be done.

(*) We can provide a blanket impl of UnsafeFuture for all Futures, and we can also provide UnsafeFuture an unsafe method to "unwrap" itself as a Future that is safe to poll.

@MajorBreakfast
Copy link
Contributor

MajorBreakfast commented Jul 13, 2019

Here's my two cents:

  • @cramertj's explanation ([Stabilization] async/await MVP #62149 (comment)) convinces me that unsafe async functions are the right design.
  • I very much prefer a fixed ordering of the keywords unsafe and async
  • I slightly prefer the ordering unsafe async fn because the ordering seems more logical. Similar to "a fast electric car" vs "an electric fast car". Mainly because an async fn desugars to an fn. So, it makes sense that the two keywords are next to each other.

@burdges
Copy link

burdges commented Jul 13, 2019

I think let f = unsafe { || { ... } } should make f safe, an UnsafeFn trait should never be introduced, and a priori .awaiting and async unsafe fn should be safe. Any UnsafeFuture needs strong justification!

All this follows because unsafe should be explicit, and Rust should nudge you back into safe land. Also by this token, f's ... should not be an unsafe block, rust-lang/rfcs#2585 should be adopted, and an async unsafe fn should have a safe body.

I think this last point might prove rather crucial. It's possible that every async unsafe fn will employ an unsafe block, but similarly most would benefit from some safety analysis, and many sound complex enough to mistakes easy.

We should never bypass the borrow checker when capturing for closures in particular.

@gnzlbg
Copy link
Contributor

gnzlbg commented Jul 15, 2019

So my comment here: #62149 (comment) is a very bad idea.

An UnsafeFuture trait would require the caller to write unsafe { } to poll a future, yet the caller has no idea of which obligations must be proven there, e.g., if you get a Box<dyn UnsafeFuture> is unsafe { future.poll() } safe ? For all futures ? You can't know. So this would be completely useless as @rpjohnst pointed out on discord for a similar UnsafeFn trait.

Requiring Future's to always be safe to pol makes sense, and the process of constructing a future that must be safe to poll can be unsafe; I suppose that's what async unsafe fn is. But in that case, the fn item can document what needs to be upheld so that the returned future is safe to poll.

@Centril
Copy link
Contributor

Centril commented Aug 1, 2019

@rfcbot implementation-work-blocking-stabilization

There are still 2 known implementation blockers to my knowledge (#61949, #62517) and it would still be good to add some tests. I'm resolving my concern to make rfcbot not be our blocker time-wise and then we'll actually block on the fixes instead.

@Centril
Copy link
Contributor

Centril commented Aug 1, 2019

@rfcbot resolve implementation-work-blocking-stabilization

@rfcbot
Copy link

rfcbot commented Aug 1, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Aug 1, 2019
@Centril
Copy link
Contributor

Centril commented Aug 2, 2019

Filed stabilization PR in #63209.

@rfcbot
Copy link

rfcbot commented Aug 11, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Aug 11, 2019
@RalfJung
Copy link
Member

An interesting aspect that we are stabilizing here is that we are making self-referential structs available from safe code. What makes this interesting is that in a Pin<&mut SelfReferentialGenerator>, we have a mutable reference (stored as a field in the Pin) pointing to the entire generator state, and we have a pointer inside that state pointing to another piece of the state. That inner pointer aliases with the mutable reference!

As a follow-up to this, @comex actually managed to write some (safe) async Rust code that violates LLVM's noalias annotations the way we currently emit them. However, it seems due to the use of TLS, there are currently no miscompilations.

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 AsyncAwait-Polish Async-await issues that are part of the "polish" area disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.