[clang-tidy][RFC] checks concurrency-async-{blocking, fs, no-new-threads}


With official C++20 inclusion of coroutines it gets important to identify synchronous code usage inside of coroutines. Volunteer preemption of a system thread in asynchronous code (e.g. in
coroutines/fibers/green threads) is a bug that prevents the current thread from executing other coroutines/etc. and negatively affects overall process performance.

I've tried to address it in a series of clang-tidy checks that
try to find such synchronous code based on blacklists from
POSIX/C++ std/C11/Boost/Linux syscalls. Basically, one should enable concurrency-*
checks for the strictest mode. It finds usages of synchronous primitives (e.g. std::mutex), fs access (e.g. open(3)), implicit system threads creation (e.g. via std::execution::par). In some cases one may want to relax the check, e.g.:
- if new threads creation is not an issue, I don't force coroutine-only code, just don't want to block coroutine threads => disable concurrency-async-no-new-threads
- if I have no means to delegate fs access to a thread pool and don't want to use aio, so I have to make synchronous fs access from coroutines => disable concurrency-async-fs

Nathan James in a review (https://reviews.llvm.org/D94621#2497859) questions whether such check based on functions/types blacklist worth implementing. I think it does as every coroutine/fiber-based C++ program may suffer from the same problem - you may not use a lot of std stuff unless you're OK with ineffective code, so having such "do not use part of std/boost" checks is handy.

The checks:

concurrency-async-blocking: ⚙ D93940 [clang-tidy] Add a check for blocking types and functions.
concurrency-async-fs: ⚙ D94621 [clang-tidy] add concurrency-async-fs
concurrency-async-no-new-threads: ⚙ D94622 [clang-tidy] add concurrency-async-no-new-threads

The patch series are based on a much more simple check used in Yandex.Taxi. I think it worth something to the community, so I've separated a big list of functions/types apart to address e.g. environments without fs threadpool, and added possibility to extend the types/functions lists via option.

I'd be happy to hear any comments how the checks can be improved, or maybe organized in some other way, or whether they confront any implicit clang-tidy policy.


I think this is definitely a real problem and it would be great to have a solution but I am somewhat sceptical that clang-tidy is the right tool to address it. The problem is that it's not just blocking system calls that are unsafe, it's anything that transitively calls those functions.

To have a reliable tool, you'd want something (possibly built on the static analyser? I don't know what the status of cross-module analysis) that would build a call graph and warn when a code path might end up in a blocking call (ideally by analysing which code paths are guarded on specific argument values).

You could probably approximate this with an incremental clang-tidy pass that would report functions that called functions in a list and updated the list with additional things that it had found.

It is possible that coroutine code in the wild isn't doing many deep function calls as more general code, in which case it's quite plausible that a simple clang-tidy check would be sufficient. I worry that a deny-list approach is going to be far less maintainable than an allow-list though: in POSIX there are a handful of system calls that can block, but in win32 there are vastly more and higher-level APIs such as Qt have a massive API surface.

A list of C++ standard-library functions that are safe to call in coroutines seems plausible to maintain. I wonder if the right approach is a [[clang::coroutine_safe]] attribute that we can put on libc++ functions, methods, and classes (if all methods are safe to call, or with a [[clang::coroutine_unsafe]] variant for methods where the default has been defined). We could then have a checker that warns if you call a function that isn't marked as coroutine-safe from a coroutine or from a function that is marked as coroutine-safe.