search

OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.43k stars 2.4k forks source link

ILocalClock.LocalNowAsync causes a VSTHRD003 code analysis warning #16751

Closed rjpowers10 closed 1 month ago

rjpowers10 commented 1 month ago

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 commented 1 month ago

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

hishamco commented 1 month ago

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