search

madskristensen / Community.VisualStudio.Toolkit

A community toolkit for writing Visual Studio extensions
Other
24 stars 3 forks source link

Prefer AsyncPackage.JoinableTaskFactory when running async tasks instead of ThreadHelper #7

Closed bluetarpmedia closed 3 years ago

bluetarpmedia commented 3 years ago

My understanding is that best practice is to use the AsyncPackage JTF when running async tasks instead of ThreadHelper so that VS can ensure they finish/cancel correctly before shutdown.

"But you generally should prefer your own AsyncPackage.JoinableTaskFactory instance over ThreadHelper.JoinableTaskFactory so that any async work you execute will be tracked and will block IDE shutdown till its done." -- Andrew Arnott (https://github.com/NuGet/Home/issues/8317#issuecomment-510304886)

See also: https://github.com/microsoft/vs-threading/blob/main/doc/cookbook_vs.md#how-do-i-effectively-verify-that-my-code-is-fully-free-threaded

madskristensen commented 3 years ago

It makes sense to use that whenever an instance of a Package is passed on to the VSSDK Helpers, such as the command base class. Are there any particular other places you've noticed could benefit from this?

Update: Just saw your PR and merged it. Thanks 👍

bluetarpmedia commented 3 years ago

It was just CommandBase that uses RunAsync. I can imagine in the future a design decision may be needed for APIs that need to call JTF.RunAsync if they don't already have the package. Should the client pass the AsyncPackage as an argument to the API, or the class ctor, or will the API just use ThreadHelper.JTF internally? My vote would be for the former.

yannduran commented 3 years ago

I also like the idea of passing/storing the package whenever possible