microsoft / microsoft/vs-threading
Add thread affinity labeling attributes
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 1k
- Forks
- 160
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 28
Description
Problem statement
Our newly data-driven VSTHRD010 rule still requires that when any individual type becomes free-threaded, that it must opt out of the namespace rule via an out of band data file. It also doesn't lend easily to affinity flags that vary depending on the version of VS being targeted.
By improving thread affinity labeling of types in the VS SDK, we can increase the confidence that VS and VS extension developers have in themselves when moving code to background threads, increase the likelihood that they do so, with fewer bugs.
Proposed solution
Let's define custom attribute(s) in Microsoft.VisualStudio.Threading.dll that other libraries can use to self-identify as thread-affinitized or free-threaded. When attributes are present, these attributes take precedence over the AdditionalFiles that otherwise help VSTHRD010 understand whether a type or member has thread-affinity. This carries benefits including:
- It's more clear to someone during a code review
- Can be changed at any time to match code changes and is more likely to be kept current.
- Can vary between versions of a consuming library without worrying about partitioning the
AdditionalFilesdata based on version of the library.
Specifically, here is the public API we would add:
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface | AttributeTargets.Event | AttributeTargets.Property | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Assembly)]
public class InvokeOnAttribute : Attribute
{
public InvokeOnAttribute(ThreadAffinity threadingRequirement);
public ThreadAffinity ThreadingRequirement { get; }
}
public enum ThreadAffinity
{
MainThread,
AnyThread,
}
We allow the attribute to be applied to any members that can be invoked (methods, properties, events) of course, since these carry code that may be thread-affinitized. But we also allow the attribute to be applied to types and assemblies as a way of setting the default thread affinity when no more specific attributes applied to the member in question.
When an individual member has no attribute, our VSTHRD010 analyzer will search for one following this order, stopped at the first match:
- On base members that this one overrides (implicitly, since this attribute is inherited)
- On the matching member of each interface that this member implements, when applicable
- On each interface that defines the member being implemented, when applicable
- On the assembly that defines the interface being implemented by the member
- On the declaring type
- On the base type, or the assembly that defines it (recursively for all base types).
- The containing type (recursively)
When in the above list we reference an interface being implemented, when more than one interface is implemented by a given member, our search order is alphabetical on the full type name.
❓ Should the above list be searched be satisfied based on data from our AdditionalFiles, or strictly only on the use of an attribute? Should attributes always take precedence over AdditionalFiles, or only when certain locality requirements are met?
The VSTHRD010 analyzer would not care whether the attribute is defined by the Microsoft.VisualStudio.Threading.dll library or another one. The attribute may be internal with respect to another library too. This way a library can declare its own thread affinity requirements without taking an assembly reference dependency on the vs-threading library if that isn't desirable for some reason.
Describe alternatives you've considered
We could continue investing in the data file, developing MSBuild targets that can detect the version of VS SDK being targeted and supply different AdditionalFiles items based on that.
A downside to using attributes is if a type or member is mislabeled, fixing it requires an in-band code change and re-shipping the SDK for that library. If all such data is out of band (shipping in the vssdk-analyzers package, for example), fixing an error is a much simpler matter (although it requires that everyone update their package references either way).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the VSTHRD010 analyzer, its AdditionalFiles handling, and the Microsoft.VisualStudio.Threading.dll API surface described in the issue. Determine the attribute precedence and member/type/assembly lookup rules, then add the proposed API and analyzer behavior with tests demonstrating the search order and completion criteria.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- devtools, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100