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

ILocalClock.LocalNowAsync causes a VSTHRD003 code analysis warning #16751

Closed
rjpowers10 opened this issue Sep 18, 2024 · 2 comments · Fixed by #16752
Closed

ILocalClock.LocalNowAsync causes a VSTHRD003 code analysis warning #16751

rjpowers10 opened this issue Sep 18, 2024 · 2 comments · Fixed by #16752
Labels

Comments

@rjpowers10
Copy link
Contributor

Describe the bug

Calls to ILocalClock.LocalNowAsync causes a VSTHRD003 code analysis warning.

Avoid awaiting or returning a Task representing work that was not started within your context as that can lead to deadlocks.

Orchard Core version

Orchard Core 1.8.2

To Reproduce

Steps to reproduce the behavior:

  1. Add a package reference to Microsoft.VisualStudio.Threading.Analyzers version 17.11.20
  2. Call ILocalClock.LocalNowAsync

Expected behavior

No code analysis warning is thrown.

I'm not super knowledgeable in this area but I suspect the issue is because the LocalNowAsync property isn't actually async. As far as I know async properties are not a thing in C#. Making this an async method would probably resolve the issue. Either that or this is a false positive, but I don't know enough to say.

The implementation of the property simply calls the private GetLocalNowAsync method. Presumably if that method was made public I could call it instead and avoid the warning.

@Piedone
Copy link
Member

Piedone commented Sep 18, 2024

This looks just like a mistaken design decision. We shouldn't really have properties returning Tasks.

@hishamco
Copy link
Member

Right, as I said in the related PR this is the first time I saw async property!! I'm not sure if there's reason for that

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

Successfully merging a pull request may close this issue.

3 participants