mono / monodevelop

MonoDevelop is a cross platform .NET IDE
http://www.monodevelop.com
2.84k stars 1.02k forks source link

[Ide] Load AddinManager data on the UI thread. #9391

Closed Therzok closed 4 years ago

Therzok commented 4 years ago

We need to run this on the UI thread since AddinManager is not thread safe. Invoking extension changed handlers on non-UI thread can lead to weird results

Fixes VSTS #1027414 - [FATAL] SigTerm signal in Mono.Addins.dll!Mono.Addins.RuntimeAddin::LoadModule+82"

Therzok commented 4 years ago

Deadlocks in TextEditor and VersionControl.Git

Therzok commented 4 years ago

One other solution I can think of is preloading the addin assemblies somewhere else (read earlier in the startup sequence), and avoiding possible deadlocks. cc @slluis what do you think?

Therzok commented 4 years ago

I'll open another PR where we eagerly load the addin assemblies that are required for the composition and we get rid of the main thread synchronization here. It might slightly regress startup time, but the cost of reliability is higher.

With this version of the PR, we can fail to initialize the IDE/extensions if extensions query MEF before it's built. By having the on-demand behaviour of the lazy init, we just eagerly load the addin assemblies from the extension point and start composing MEF in the background right after that happens. Using that mechanism, we should be able to get the best of both worlds:

Therzok commented 4 years ago

Running 2 CI builds, one with changes in tests too, one without, with the v2 fix.

Therzok commented 4 years ago

Something's wrong with the test runner it seems. If an exception is thrown, it hangs.

slluis commented 4 years ago

@monojenkins backport release-8.4