Warning when calling a blocking function in an async context

A common issue users are having with async code is calling blocking functions inside of async contexts and being surprised that they block.

While we can't perfectly solve this problem, it would be fairly straightforward to have a heuristic lint.

First, it would require a new attribute similar to #[must_use], let's say #[might_block]. This would only apply to function/method items. We'd apply this to every function in std which we consider to be "blocking" (because it performs blocking IO or blocks the thread for synchronization), and we'd make it available on stable for people to apply to their own functions.

Then, if the user calls a function tagged might_block from inside an async context, we issue a warning them that this might block. This warning could be supressed, made into an error, etc.

As a more extreme solution (possibly a second step), we could make this attribute transitive, so any function which calls a might_block function is also might_block, unless its been tagged that it shouldn't be considered blocking for whatever reason (e.g. #[allow(might_block)]).

The attribute could also have an option for specifying a nonblocking alternative, though for std of course there are no nonblocking alternatives available in std.

35 Likes

I guess it's not really feasible to make the output of a suggestion dependent on whether someone has async-std as a dependency, then suggesting the appropriate async-std function, if it exists?

We don't integrate that tightly with unofficial crates that the Rust project doesn't maintain (or pick favorites: even though async-std's APIs are more direct parallels to std's, tokio provides the same functionality).

EDIT: We could have some hook for dependencies to inject their own APIs into these lints (eg the suggestion would come from async-std or tokio directly), but that's getting very complicated and definitely out of scope for an MVP.

5 Likes

Yea, that makes sense. This proposal sounds really cool, but definitely out of scope for now :wink:

1 Like

I'm in favor of an attribute like this. Using a blocking function inside an async context is a very inconspicuous error. Blocking functions are very common both in the standard library and in the whole ecosystem. Using them in an async context compiles without warnings, it passes all your tests, it even runs fine in production as long as you don't actually stress the system. You could write an http server with blocking code in its async functions that runs fine for months until one day you get a traffic spike and only then performance falls off a cliff.

And even then, you might have a very hard time finding that the root cause of your poor performance is that you used some blocking functions somewhere in your entire application. It might not even be you who does the blocking, it could happen in some unexpected place in a library somewhere. And even if you suspect blocking calls to be the issue, finding these blocking calls doesn't sound easy either.

It is really the worst kind of error: easy to make, hard to detect (both in the code and at runtime), and hard to debug, so an attribute like #[might_block] would be very useful. It makes the error harder to make and easier to detect. This attribute will never be able to catch all mistakes without changes to the type system (e.g. Option::map should be #[might_block] if and only if its argument is #[might_block]), but it's good enough even if we just catch some of the mistakes, both as a teaching tool for new users who might not be aware of this issue, and as an extra line of defense against slip ups by experienced users.

I agree that #[might_block] should theoretically be transitive, in the sense that using a #[might_block] function inside another function (not marked #[might_block]) should give a warning that you should also mark that function #[might_block]. But from a practical point of view, I'm not so sure, because that would create lots and lots of warnings, nagging people to mark functions as #[might_block] even though they were never intended to be used in an async context. Perhaps this transitive warning should only be applied to pub functions in libraries. (EDIT: maybe this transitive warning should just be a clippy lint?)

I'm also not sure how pedantically #[might_block] should be applied to functions that can technically block but rarely do. As mentioned in this blog post, even println!() can technically block, but the times where that will actually cause an issue seem to be extremely rare. I think adding #[might_block] to println!() would not be a good idea.

4 Likes

What I've run into recently was that I accidentally had a parking_lot::MutexGuard across an .await and it caused a deadlock (or actually panic because this was in wasm). An idea that I had would be that blocking guard-like data structures could be annotated with

#[cant_yield]
struct MutexGuard<T> { ... }

which would error out / warn if a value of such a type ends up in the backing generator states of the async fn / generator.

4 Likes

I meant the compiler would track this and warn end users, not that they would warn for not tagging the functions might block. Indeed, the idea of making it transitive is that users would not have to tag all their functions.

Your point about Option::map is good: its possible this would not work at all with higher order functions. However, its also possible that we might be able to run this lint after instantiation, when the type is known, in which case an intelligent enough "transitive" rule would even catch those. Probably not a requirement for the MVP though.

This is interesting! I thought about tagging just lock to catch this case, but if you never lock across a yield point or blocking function, locking itself is not really the problem. You're right that an attribute to warn when certain objects are alive across a yield point would be helpful.

I like the idea of a transitive #[might_block] because it lets us treat OS-level blocking (or whatever we apply it to) as an effect. Current effect-like things in Rust are manually annotated at all levels (async, try/Result, const), but effects can also be inferred, even polymorphically, so we wouldn't even need to run the lint post-instantiation if it were implemented this way.

3 Likes

I really like the "cant_yield" idea, but I'm not sure how feasible the "might_block" version is...

How do you know whether you're in an async context? Async functions probably call other functions and those functions might block. Does rustc have to analyse the transitive call graph of every async function? What about when the Future trait is implemented directly?

Also, it's not necessarily an error to block in an async function, it depends on the executor/what else is running.

1 Like

cc @eddyb and @Zoxc re. the transitive idea; I wonder how much of a hit that would be for compile times.

I think we shouldn't make it transitive below the crate level. That way, we can have a lint that marks all exported non-#[might_block] functions that call into #[might_block] functions transitively – we should have the complete call graph of a crate available anyway.

This would reduce the burden of annotating to public functions and keep our analysis crate-local.

5 Likes

Do I correctly understand that can be easily extended? IE with attributes?

Well that was a great point. But it implies if I maintain defaults and link crates a lint could read it. In a seperate crate of course.

Of course. You can't argue the block is not "inherited" generally. Provided that you can override that and otherwise not break anything. IE Still retains your current flexibility.

Maybe this should be generalized to a general effect system, i.e. have #[might(Block)] instead of #[might_block] where Block is an effect.

5 Likes

I'm not fully convinced, but I think additional tooling around this kind of problem could set Rust apart from other languages. A danger I see is ending up with something that people might take for authoritative, while it can really ever be a best effort. There's a danger of building a feature that people rely on and get frustrated when something blocks that wasn't marked.

So, what's necessary before introducing such a feature: figuring out what a metric for "good enough" is and where we cross the boundary where this feature is useful in most cases.

What I am wondering: is it possible to encode "this future includes a blocking call at location xyz" and let an executor use that information at runtime?

I agree. I get very nervous about the expectations users will draw (and also scope creep) when we start talking about "blocking as an effect"

3 Likes

TBH, if such a feature can be designed well, I'd prefer it much over many syntax additions.

Annotations will always be framed as scope creep, while I see them as a very good capability to encode facts that can't easily be derived by the compiler.

2 Likes

Could the blocking annotation be used to make lints warning when a function's call graph depth exceeds a certain number of blocking annotated functions? Or is that naive and not actually useful in performance optimization?

Could the blocking annotation be specified with an optional low/high modifier where adding the values of an entire call graph (blocking+modifier calls blocking+modifier calls..) becomes helpful to the developer in optimizing queue assignment?

I think this is an important point, I'm glad you mentioned it. I think developers would become frustrated if they were faced with warnings in their code, but had no clear idea about how to address them.

In addition to possible suggestions built directly into the attribute, could we consider a new chapter in the rust async book that covers this topic? (stjepang's recent blog post on this topic is also a great intro)