Load extension host after workbench is running and introduce phase

[bpasero]

I think we should try again to spawn the extension host only after the workbench is running. I remember @jrieken tried this but then we had clients that would prevent the restoring of editors from happening (via preferences editor).

We should replace IExtensionService.onReady() with a LifecyclePhase that is always fired after LifecyclePhase.Running imho and then we have a consistent picture of phases.

That means revisiting each user of IExtensionService.onReady() and making sure it is not blocking the startup. One challenge is that it is hard to prevent deadlocks from happening because we will always add new code that might cause it.

Thoughts?

[bpasero]

@sandy081 problematic in settings/keybindings: KeybindingsEditorModel.resolve() and PreferencesService.resolve()

Potentially dangerous (depends on usage): IExtensionService.getExtensions() and IExtensionService.readExtensionPointContributions()

And then a lot in IModeService: getOrCreateMode, getOrCreateModeByLanguageName, getOrCreateModeByFilenameOrFirstLine, et al

[jrieken]

Thoughts?

We should remove IExtensionService#onReady. Maybe make it a lifecycle event but surely never block on it but treat it like an event, e.g. to update something

[bpasero]

@jrieken yeah I had that thought as well, if we make it as an event it reduces the chance that someone would chain that promise up all the way to some critical code path.

However, if we have an event, that would be an argument against adding it to the lifecycle service, because there we typically have promises.

Problem I see is with the mode service things blocking on the extensions to be ready. Maybe @alexandrudima could chime in if that could also be event based and not blocking.

[sandy081]

Fixed it in Settings editor by removing the dependency on IExtensionService#onReady instead listening to default configuration changes - #38329

[sandy081]

Re KeybindingsEditorModel.resolve(), Keybindings editor renders Keybindings and unbound commands. There is already an event that is triggered when keybindings are changed / registered. But not sure there is an event triggered when commands change / registered so that the editor can be updated.

[alexdima]

@bpasero IModeService.getOrCreateMode() blocks on the extensions ready (i.e. extensions scanned and extension points invoked, not necessarily extension host process ready), but that is IMHO inconsequential as it does not block the creation/displaying of a buffer.

[bpasero]

@sandy081 well, the idea is to not join your promise on the extensions ready promise. So the event that you are looking for is still the same ("extensions ready") but you should simply update the editor when it happens instead of waiting for it before the editor is declaring to be loaded.

...I think all of this will be much easier to not do wrong once we replace the promise to be an event.

[jrieken]

There is already an event that is triggered when keybindings are changed / registered.

Really, where? I'd copy from it

But not sure there is an event triggered when commands change / registered so that the editor can be updated.

Doesn't exist but can be added

[sandy081]

@jrieken here

[sandy081]

the idea is to not join your promise on the extensions ready promise

@bpasero Not sure what you mean here. Settings editor is no longer depending on the extensions ready promise. Also it does not wait for any. It simply updates the editor with newly registered default settings by listening to the configuration change event. More details are here #38329.

I would like to do the same for Keybindings editor. But I am missing an event when commands are registered.