YieldSafe auto trait

[krojew]

Rendered

[RustyYato]

Rendered

[Diggsey]

I think there are several problems that are not addressed in the RFC.

Firstly, the example given "don't lock a mutex in an async function" is different from the prior art in C# "don't await whilst a mutex is held/in a critical section" and C# has got it more correct. Locking a mutex is fine in async context as long as you don't yield whilst it's held, and there is a lot of existing code which does this.

Secondly, the RFC doesn't explain how this attribute would be propagated across function calls. Async code often calls non-async code, and that non-async code may call a function with the #[no_async] attribute, but there's no obvious way for the compiler to detect this.

I believe a better option would be to talk about what types of value should be allowed to exist on the stack in an async function. We could imagine a new default marker trait YieldSafe, which types like MutexGuard would opt-out of, and would be inherited (so any type owning a MutexGuard would also opt-out of this marker trait). Then, the compiler would be able to show a warning whenever an async function or generator does not implement YieldSafe.

[krojew]

You are right - C# got it more correct, although I thought a more defensive approach was acceptable. A marker trait might be a better solution - then the compiler should emit a warning/error when encountering .await if something without it is on the stack. Will update the RFC to reflect that.

[steffahn]

async fn foo<T>(value: T) {
    // potentially valid code, since value doesn't need to cross .await if it doesn't implement Drop (recursively)
    bar().await
}

I don’t know if it’s just a lack of creativity on my side but I can’t think of a setting that provides a non-YieldSafe type (that is non-YieldSafe for a reason) that doesn’t implement Drop (recursively).

[mikeyhew]

This reminds me of std::panic::UnwindSafe, in that it isn't required for memory safety, but helps avoid logic errors. Maybe it would be helpful to have an AssertYieldSafe wrapper similar to AssertUnwindSafe?

[krojew]

I don’t know if it’s just a lack of creativity on my side but I can’t think of a setting that provides a non-YieldSafe type (that is non-YieldSafe for a reason) that doesn’t implement Drop (recursively).

Me neither, but it still would be possible to create one in practice.

Maybe it would be helpful to have an AssertYieldSafe wrapper similar to AssertUnwindSafe?

That seems like a good idea (assuming understanding the consequences, that is). I will add it to the RFC.

[clarfonthey]

Potentially silly question, but is there really no relationship between YieldSafe and Send + Sync?

[krojew]

Not really, since you can have a single-threaded runtime, which doesn't require futures to be neither Sync nor Send, yet still requiring YieldSafe.

[clarfonthey]

Okay, after thinking about it it makes a bit more sense, so, I'll share the way I thought about it in case it helps anyone else:

• Send is very explicit about thread synchronisation and is there to avoid data races. So, it's separate from async entirely.
• Sync is not necessarily thread-specific, as it means to clarify that there is no interior mutability. There's nothing inherent to it that can't be applied to async use cases, but it may be worth clarifying that parallel execution isn't required for data races to occur, and that this must be considered.
• YieldSafe basically guarantees that the type does not contain any synchronous locks. In other words, code should not rely on its destructor being run in order to avoid deadlock.

Taking this in mind, I think a better name might be LockFree, to indicate that the type does not contain any synchronous locks that might cause issues across an await boundary (i.e., pausing execution without calling the destructor). Additionally, because the implementation of LockFree does not affect whether data races occur, I think that it should be clarified that this is a safe trait, unlike Sync and Send.

[steffahn]

@clarfon you’re not quite there yet with your intuition. What you might have missed is the strong relationship between Send and Sync. It is perhaps best demonstrated by the fact that T: Sync if and only if &T: Send.

continuation of rather long and somewhat off-topic answer

Hence in particular it would not make sense to evaluate for `Sync` and `Send` separately whether they have something to do with `async`.

Sync is also not about interior mutability per se. Types without interior mutability tend to be Sync, but so is Mutex or RwLock. Furthermore it is rather uncommon for types to be Sync without being Send (expecially if the type is Copy, because then you could already send “by-copy”) though there are exceptions. Maybe an intuition that works often is that !Sync tend to contain un-sync-ronized interior mutability.

But the real meaning of these traits is (admittedly from a quite formalist point of view): They are unsafe marker traits that allow the standard library have a sound API; in particular, the contract on these traits seems to be that Send types are the ones that can be sent between threads and Sync types are the ones that are allowed to be referenced from different threads at the same time, in other terms, references to Sync types can be aliased between threads.

Soundness of an API in rust means that without using unsafe it is impossible to break basic guarantees from the Rust programming language by using that API. Guarantees such as: it is impossible to trigger undefined behavior (which is not quite true currently, unfortunately), there’s no double-frees, no segfaults, no data-races.

Back to the topic of YieldSafe: This trait would not be about soundness but be there to help avoid logic errors. In that regard it is similar to UnwindSafe with which it shares the naming scheme, apparently. They both are, practically, not much more than a lint that doing something specific may be problematic if the type lacks the trait: in the case of UnwindSafe the problematic thing is catching and inspecting the type as payload of a panic (there’s a RefUnwindSafe that acts a bit like Sync does to Send regarding references). It’s basically just like a lint since there are no strict guarantees coming with the trait (you could always leak stuff out of caught panics using a RefCell), hence it is not unsafe.

For YieldSafe the problematic action is storing the type inside the state of a Future (where the state is referring to the information that needs to be saved during an await).

I’m with you on the point that this could be even more explicit that YieldSafe is not an unsafe trait, even though the text makes it quite clear already: “While it's true such dangerous operations are logic errors, not related to language itself, it would be very helpful if there was a mechanism to mark types as potentially hazardous to use in async code ...”

TLDR (and a conclusion): Safe and Sync are deeply connected to each other. Implementing them follows rules/contract to ensure soundness and is hence unsafe. Approaching them with intuition only is not that easy and may lead to wrong conclusions. YieldSafe is not about soundness but logic errors. It is essentially a way for a library author to enable a warning on probably improper use of the type. Giving YieldSafe a different meaning based on intuition, like LockFree can be misleading, too, since it’s true meaning is simply (when it’s not implemented): The author of this type (or its components) does not recommend you to keep it around in an idle Future during await.

Also now I’m wondering, @krojew, what do you think about a RefYieldSafe trait?

[krojew]

At the moment, I'm not sure RefYieldSafe is necessary. We can assume &T is YieldSafe if T is, so what would be the use case? Is there one for a negative impl when some T is YieldSafe?

[krojew]

Are there any more questions about this RFC?

[withoutboats]

Thanks for this RFC. The async foundations working group is working on a must_not_await lint, which is intended to meet the same purpose, but using a lint attribute instead of an auto trait. rust-lang/wg-async#16

We'd like to consolidate work on this area into a single proposal, so we're inclined to close this in favor of the proposal from the WG. If there are important differences between the two proposals, it would be great for people to hash that out on the async foundations WG's proposal and possibly modify that proposal as needed.

@rfcbot fcp close

[rfcbot]

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

• @cramertj
• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @withoutboats

No concerns currently listed.

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.

[Ekleog]

@withoutboats I do not understand how to interact in a WG discussion, and given my experience trying to do so I won't have enough energy to try again.

Would it be possible for you (or anyone else) to raise at the new right place the fact that auto traits as suggested here, which have no arguments listed against on the WG document, also would naturally solve the question that is currently marked as “unresolved” there? I wouldn't have to add beyond this one point.

[rfcbot]

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

[rfcbot]

The final comment period, with a disposition to close, 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 is now closed.