Page MenuHomePhabricator

[clang-tidy] add concurrency-async-fs
Needs ReviewPublic

Authored by segoon on Wed, Jan 13, 11:07 AM.

Details

Summary

Search for filesystem accesses that might block current system thread.

The check tries to find all fs-related types/functions from the
following list:

  • C++ std
  • POSIX
  • Linux syscalls
  • Boost.Filesystem, Boost.Nowide

Custom functions and types can be added via options.

Diff Detail

Event Timeline

segoon requested review of this revision.Wed, Jan 13, 11:07 AM
segoon created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Jan 13, 11:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
segoon updated this revision to Diff 316460.Wed, Jan 13, 11:08 AM
segoon retitled this revision from docs to [clang-tidy] add concurrency-async-fs.
segoon edited the summary of this revision. (Show Details)

fix

Is this just flagging all these functions, if so I don't think there's much value in here.

njames93, the purpose is to flag it indeed. This approach was found valueable in Yandex.Taxi, as it is very easy to forget that you're in a coroutine and may not use blocking API. The bug does affect performance (e.g. all coroutine threads wait for fs), it cannot be found by functional tests (as it is not a functional invariant violation) and may be rather tricky to debug (as the performance harm depends on many things like IO limits, page cache size, current load, etc.). It can be caught during code review, but it suffers from human errors. Rather than playing catch-me-if-you-can games, the check can be automated. As C/C++ standard libraries contain quite many blocking functions and C++20 gains official coroutine support, I find it valuable for the C++ community to have an already compiled list of such blocking functions and the check that uses it.

Eugene.Zelenko added inline comments.
clang-tools-extra/docs/clang-tidy/checks/concurrency-async-fs.rst
7

Please make first statement same as in Release Notes.

11

Please highlight aio and io_uring with single back-ticks.

16

Please highlight tmpfs with single back-ticks.

33

Please add information about default value. Same below.

Eugene.Zelenko set the repository for this revision to rG LLVM Github Monorepo.
Eugene.Zelenko added a project: Restricted Project.
segoon updated this revision to Diff 316667.Thu, Jan 14, 8:07 AM
  • use back-ticks
  • fix the first document line
  • add default values

Eugene.Zelenko, thanks for the review! fixed

Eugene.Zelenko, njames93, any comments on the patch?