microsoft / microsoft/vs-threading

Analyzers for lock primitives that need thread affinity

Open
#1,313 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

analyzers enhancement
Dominant language
C#
Stars
1k
Forks
160
Avg merge
1d 12h
Merged PRs (30d)
28

Description

Is your feature request related to a problem? Please describe.

I have been advocating for vs threading analyzers, which are awesome to enforce proper async semantics and prevent common anti-patterns.

Unfortunately, while fixing the many violations in a service codebase, a subtle regression was introduced inside a lock scope using ReaderWriterLockSlim which was changed from sync-over-async to proper async calling pattern. Needless to say, the code start throwing exceptions when using this lock.

Given ReaderWriterLockSlim relies on thread affinity and the code did not have enough test coverage to detect the regression, the issue went undetected and resulted in a bad outage.

Describe the solution you'd like

Keeping aside that the correct fix is to use an async lock such as AsyncReaderWriterLock I'd love to see a static analyzer rule to detect misuse of async inside lock scopes that rely on thread affinity such as ReaderWriterLockSlim. Ideally such rule would be bundled in VS Threading analyzers, so this type of regression can be avoided in the future, especially when adopting the VS threading analyzers itself, one would hope it helps reducing issues in code, not adding.

Describe alternatives you've considered

I don't see alternatives to prevent such mistakes at compile time.

Additional context

C# lock​ blocks disallow await​ expressions within them for this reason but analyzers could do the same for other thread-affinitized primitives. Some impacted classes:

  • Monitor
  • ReaderWriterLock(Slim)
  • Mutex

cc: @AArnott

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the linked VS Threading analyzers documentation and reviewing the existing analyzer rules. The issue names Monitor, ReaderWriterLock(Slim), and Mutex as affected primitives, but no files or tests are specified. Done means detecting async misuse inside these thread-affinitized lock scopes with coverage for the described regression.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.