gracefully terminate plugin host process without rpc connection

[akosyakov]

What it does

fix #7176, fix #5019, fix #5839, fix #6677, fix #7113

The issue arises when there are pending requests in the plugin host process and the main side is already gone (which is basically always on reload). For some reasons we create a new RPC protocol which is not capable of handling such pending requests to stop plugins.

Instead of creating new protocol this PR adds new control messages:

• terminate to ask the plugin host process to shutdown rpc connection and deactivate all plugins as much as possible
• terminated to get notified when the host process is done to kill it and its children
• all other communications are prohibited during termination, since there is nobody to talk to and it will for sure fail

It fixes the issue with massive amount of error logging produced on each reload, around 2K lines.
For instance, I checked Gitpod logs for last 7 days and first 3 of 5 most frequent errors are incarnation of this issue.

It as well improves stopping of plugins, since deactivation of VS Code extension can happen asynchronous, but we were not handling it.
In addition, now the plugin host process has a chance to kill child processes on its own (by disposing RPCProtocol) without relying on Theia backend.

How to test

• Reload the page while causing extensive communication with the plugin host process, i.e. debug something, use language features causing many diagnostics and so on.
• Check that backend logs don't have any errors about unknown actors or closed connections from the plugin host process. There can be some errors from concrete plugins though, but they should be rare.
• Check that the plugin host process is shutdown and all its children, i.e. if you debug something, you should debug it again on reload, ports should not be occupied. Use system tools to explore process trees, i.e. ps -auxf on linux.

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[akosyakov]

@kittaakos Could you check please whether you can reproduce #7113 with it?

[svenefftinge]

LGTM,
I have tested with the typescript extension by creating a very large file with lots of errors and reloading after applying changes and hitting content assist. Did that multiple times. The LS always reconnected and worked properly. In the terminal I saw no messages about unknown actors or closed connections and no dangling processes.

I still get the errors about the missing registerSelectionRangeProvider, which I guess is expected and not part of this PR. Also old node processes are shown as in ps.

[kittaakos]

I have verified it locally in electron with the vscode-go, vscode-yaml, and TS LS.

• Error: kill ESRCH when starting, restarting frontend app with the go and yaml VS Code LS extension #7113 is OK ✅
• Error: Unknown actor YourServiceName when refreshing the browser window #7176 is OK ✅

Besides the Possible Emitter memory leak detected warnings when opening a JS file from the node_modules (also logged here), I do not see any errors in the log. Very nice 👍Thank you!

root ERROR [hosted-plugin: 38386] Error: Possible Emitter memory leak detected. 31 listeners added. Use event.maxListeners to increase the limit (30)
    at Emitter.checkMaxListeners (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:253:26)
    at PreferenceRegistryExtImpl._event.Object.assign.maxListeners [as onDidChangeConfiguration] (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:221:27)
    at Object.onDidChangeConfiguration (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-context.js:387:46)
    at w.start (/Users/akos.kitta/git/theia/plugins/vscode-builtin-jake/extension/dist/main.js:1:4299)
    at t.activate (/Users/akos.kitta/git/theia/plugins/vscode-builtin-jake/extension/dist/main.js:1:6063)
    at PluginManagerExtImpl.<anonymous> (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:548:87)
    at step (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:47:23)
    at Object.next (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:28:53)
    at fulfilled (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:19:58)
root INFO [hosted-plugin: 38386] PLUGIN_HOST(38386): PluginManagerExtImpl/loadPlugin(/Users/akos.kitta/git/theia/plugins/vscode-builtin-gulp/extension/dist/main)
root ERROR [hosted-plugin: 38386] Error: Possible Emitter memory leak detected. 32 listeners added. Use event.maxListeners to increase the limit (30)
    at Emitter.checkMaxListeners (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:253:26)
    at PreferenceRegistryExtImpl._event.Object.assign.maxListeners [as onDidChangeConfiguration] (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:221:27)
    at Object.onDidChangeConfiguration (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-context.js:387:46)
    at b.start (/Users/akos.kitta/git/theia/plugins/vscode-builtin-gulp/extension/dist/main.js:1:4494)
    at t.activate (/Users/akos.kitta/git/theia/plugins/vscode-builtin-gulp/extension/dist/main.js:1:6258)
    at PluginManagerExtImpl.<anonymous> (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:548:87)
    at step (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:47:23)
    at Object.next (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:28:53)
    at fulfilled (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:19:58)
root INFO [hosted-plugin: 38386] PLUGIN_HOST(38386): PluginManagerExtImpl/loadPlugin(/Users/akos.kitta/git/theia/plugins/vscode-builtin-grunt/extension/dist/main)
root ERROR [hosted-plugin: 38386] Error: Possible Emitter memory leak detected. 33 listeners added. Use event.maxListeners to increase the limit (30)
    at Emitter.checkMaxListeners (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:253:26)
    at PreferenceRegistryExtImpl._event.Object.assign.maxListeners [as onDidChangeConfiguration] (/Users/akos.kitta/git/theia/packages/core/lib/common/event.js:221:27)
    at Object.onDidChangeConfiguration (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-context.js:387:46)
    at w.start (/Users/akos.kitta/git/theia/plugins/vscode-builtin-grunt/extension/dist/main.js:1:4718)
    at t.activate (/Users/akos.kitta/git/theia/plugins/vscode-builtin-grunt/extension/dist/main.js:1:6483)
    at PluginManagerExtImpl.<anonymous> (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:548:87)
    at step (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:47:23)
    at Object.next (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:28:53)
    at fulfilled (/Users/akos.kitta/git/theia/packages/plugin-ext/lib/plugin/plugin-manager.js:19:58)

[benoitf]

looks very nice 👍

[akosyakov]

@benoitf thanks! I will update the PR and ping you again

[akosyakov]

@benoitf About configurable timeouts, env variables and cli options should work? i.e. users for them are adopters, not end users. It is not feasible to configure it via preferences since the user session is not always available (i.e. during page reload we cannot read them) and I'm not sure that it is desirable to give control over them to end users.

[benoitf]

@akosyakov yes sure I was more thinking a setting of the product (not user preferences)

[akosyakov]

@benoitf I've pushed again:

• logger is used now
• there are new ENV variables and cli options to control plugin host termination timeout as well as timeout of stopping plugin host services

[benoitf]

thanks @akosyakov . I may be able to test only this afternoon but I've no objections if it's merged before.

[akosyakov]

@benoitf I will merge on Monday

[benoitf]

👍 tested with several browser windows, all processes are properly killed after refreshing/closing all tabs