Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JoinableTaskContext.IsOnMainThread should satisfy VSTHRD010 #1100

Open
zivkan opened this issue Oct 29, 2022 · 1 comment
Open

JoinableTaskContext.IsOnMainThread should satisfy VSTHRD010 #1100

zivkan opened this issue Oct 29, 2022 · 1 comment

Comments

@zivkan
Copy link
Member

zivkan commented Oct 29, 2022

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

Previously commented in another issue: #272 (comment)

I have a method I want to be free-threaded, but I have dependencies which are not. How I implement my method depends whether it's running on the main thread, or a background thread. Copying the example from the other issue:

public void NotAffinitized() {
  if (IsOnMainThread()) {
    // This incorrectly marks the enclosing method as affinitized, which then propagates
    SomeAffinitizedMethod();
  } else {
    SomeNonAffinitizedMethod();
  }
}

Describe the solution you'd like

No VSTHRD010 within the if block.

Describe alternatives you've considered

An await jtf.SwitchToMainThreadAsync(alwaysYield: false) does suppress the warning, but feels inefficient. Even if it's fast, it's still extra work that isn't necessary for program correctness, just to satisfy the analyzer.

Additional context

@zivkan
Copy link
Member Author

zivkan commented Oct 29, 2022

Here's some test code that I believe should be made to work:

        var test = @"
using System.Threading.Tasks;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Task = System.Threading.Tasks.Task;

class Test
{
    void Foo()
    {
        ThreadHelper.ThrowIfNotOnUIThread();
        IVsSolution solution = Package.GetGlobalService(typeof(SVsSolution)) as IVsSolution;
    }
    async Task FirstAsync()
    {
        if (ThreadHelper.JoinableTaskFactory.Context.IsOnMainThread)
        {
            Foo(); // this generates a warning, even though Reset() doesn't assert
        }
    }
}
";
        await CSVerify.VerifyAnalyzerAsync(test);

However, I haven't yet been able to get the product code to make the test pass. I don't have any experience with the Roslyn APIs, so it's a learning curve to contribute. As much as I'd love to get my own name in the commit history, if someone else can get this implemented before me, I'll be happy to be able to take advantage of the improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants