Adopt new extension host restart lifecycle

[bpasero]

Synopsis
We allow the extension host to be restarted in certain cases. Today this happens without any support for asking components whether they can support a restart and so issues arise.

#180513 introduces a new IExtensionService.onWillStop event that listeners can veto(boolean | Promise<boolean>), for example:

// sync
extensionService.onWillStop(e => e.veto(true, 'component id'));

// async
extensionService.onWillStop(e => e.veto(Promise.resolve(true), 'component id'));

This allows to:

• veto the restart and thus prevent it
• prolong the restart until work has finished without preventing the restart

In addition the method IExtensionService.stopExtensionHosts() is now a Promise<boolean> and callers must await the result before proceeding. For example:

const stopped = await this.extensionService.stopExtensionHosts();
if (!stopped) {
  return;
}

Related Issues
Some components cannot support EH restart without doing some operation first before the EH goes down, for example:

• Data loss: Custom editors do not properly deal with extension host restarts #122992
• Handle EH restart gracefully to avoid notebook working copy data loss #179224

Adoption

• Notebook component should make sure dirty and untitled notebooks are saved before the EH restarts @rebornix
• Custom editor component should make sure dirty and untitled custom editors are saved @mjbvz
• Workspace trust needs to adopt the new boolean return type of stopExtensionHosts here @lszomoru
• Profiles need to adopt the new boolean return type of stopExtensionHosts here @sandy081

[rebornix]

Notebook component should make sure dirty and untitled notebooks are saved before the EH restarts @rebornix
Custom editor component should make sure dirty and untitled custom editors are saved @mjbvz

@bpasero I'm wondering if this can be done in one level up, for example, implement the saving all logic in FileWorkingCopyManager so we don't have to try to trigger saving from notebook or custom editor component.

[bpasero]

But FileWorkingCopyManager has no knowledge about extensions or the extension host restarting. I do not think it should have a dependency to extensions service or stop it from restarting.

[sandy081]

@bpasero Would like to see the end - end flow with this API - Lets say user switches the profile and one of the components veto it. In this case what does user see? If user decides to veto (cancel) it, what is expected to do by the component triggering the action to stop extension hosts. May be instead of returning a boolean value, does it makes sense to throw a cancellation error?

[bpasero]

In this case what does user see?

Nothing, the operation is stopped. Compare this with closing a dirty editor and pressing "Cancel"

If user decides to veto (cancel) it, what is expected to do by the component triggering the action to stop extension hosts.

The expectation is to then not restart the extension host and unwind anything that was done as part of the flow (if any). Here is how workspaces do it:

vscode/src/vs/workbench/services/workspaces/electron-sandbox/workspaceEditingService.ts

Lines 156 to 160 in c7581b5

	async enterWorkspace(workspaceUri: URI): Promise<void> {
	const stopped = await this.extensionService.stopExtensionHosts();
	if (!stopped) {
	return;
	}

[sandy081]

In this case what does user see?

I am more particular about the dialog shown by the components for vetoing. For example, what kind of dialog Notebooks show? This dialog shall have the information about the user action and not the internal details like extension host is being restarted. I am expecting in the profile switch scenario, the veto dialog shall say something like Would you like to save the notebook before switching the profile? If so, the stopExtensionHosts api shall take additional context as an input so that the listeners will provide the context in the dialog.

[bpasero]

Yeah I like that, maybe as part of the call to stopExtensionHosts we could pass in a string that the components can use to show as details of the dialog. Or are you suggesting to turn it around and let components return the strings and let the extension service show the dialog?

[sandy081]

maybe as part of the call to stopExtensionHosts we could pass in a string that the components can use to show as details of the dialog.

I think, this is better for the components

[bpasero]

Adding a new reason property to the event in: #182523

I think its good that a component opens a dialog because a component will have to deal with all the buttons and actions. Components can now show reason as dialog details.

[bpasero]

Thinking that the extension service should inform the user on veto, like so:

The first message is the reason for why the extension host is restarted and should read well so please adopt accordingly @sandy081 and @lszomoru .

The second details message lists the reason why the operation was blocked and should also read accordingly @mjbvz and @rebornix

[sandy081]

✅

[bpasero]

Thanks, I think we only miss workspace trust and notebooks 🙏

[bpasero]

I think this is in very good shape now. Workspace trust transition cannot be fully cancelled, but the data loss scenario is mitigated with the veto support we have.