RPC ClientProxyHandler lazy init may lead to problems when proxies have dependencies on each other

[jfaltermeier]

Bug Description:

Currently this bug is visible as this error message on the browser console of the electron example application:

Uncaught (in promise) Error: unknown document
    at DocumentsExtImpl.$acceptDirtyStateChanged (~/git/theia/examples/electron/lib/backend/plugin-host.js:5333:19)
    at ~/git/theia/examples/electron/lib/backend/packages_plugin-ext_lib_common_plugin-api-rpc_js.js:2060:71
    at process.processTicksAndRejections (VM33 task_queues:95:5)
    at async RpcProtocol.handleRequest (~/git/theia/examples/electron/lib/backend/packages_core_lib_common_index_js-node_modules_vscode-languageserver-types_lib_umd_sync_recursive.js:3576:28)

Steps to Reproduce:

1. Build and run electron example with downloaded plugins yarn && yarn build && yarn download:plugins && yarn electron start
2. Open Debugger Tools in Electron and check the console
3. You should see a similar unknown document exception

Additional Information

At the location where the error is thrown we have this code:

const data = this.editorsAndDocuments.getDocument(uriString);
if (!data) {
    throw new Error('unknown document');
}

Here DocumentsExtImpl accesses information from EditorsAndDocumentsExtImpl (so we have a dependency between those two).

The error chain is triggered like this (as far as I could see):

The electron-menu-contribution calls this.preferenceService.set('window.titleBarStyle', this.titleBarStyle, PreferenceScope.User);

This will open user-storage:/user/settings.json, write to it, and close it again via the preference-transaction-manager. This is happening very fast.

On the browser side DocumentsMainImpl and EditorsAndDocumentsMain are both reacting to this and forward to their proxies in the expected order:

EditorsAndDocumentsMain calls
this.proxy.$acceptEditorsAndDocumentsDelta(deltaExt);
because there is a new document (user-storage:/user/settings.json).

DocumentsMainImpl calls
this.proxy.$acceptDirtyStateChanged(m.textEditorModel.uri, m.dirty);
because user-storage:/user/settings.json was touched.

EditorsAndDocumentsMain calls
this.proxy.$acceptEditorsAndDocumentsDelta(deltaExt);
again, because the document is closed again.

When we debug the other side of the communication (DocumentsExtImpl and EditorsAndDocumentsExtImpl) we can see that $acceptDirtyStateChanged arrives after the second $acceptEditorsAndDocumentsDelta call, which leads to above error. The document is already removed from editorsAndDocuments.

I think the issue is that the ClientProxyHandler implementation has lazy initialization code:

theia/packages/plugin-ext/src/common/proxy-handler.ts

Line 50 in 469bd74

private initializeRpc(): void {

While the ClientProxyHandler for editors-and-documents is already initialized (because it has been called before already) the documents handler is initialized on the first acceptEditorsAndDocumentsDelta call.
This allows the second call to acceptEditorsAndDocumentsDelta to "overtake" the first one, leading to the exception.

Hacking the ClientProxyHandler to call initializeRpc() in its constructor (i.e. basically removing the lazy init) removes the exception, but I guess this is not the desired solution.

So I think we need a mechanism to sync the initialization of ClientProxyHandlers when they have dependencies on each other.
This could e.g. be

• a init listener mechanism + a way to manually initialize handlers , or
• to provide information about dependencies to the framework, so that the framework can init all of them

I've played a bit with the listener mechanism and could open a PR if desired to kick off a discussion about the best solution.

• Operating System: Linux
• Theia Version: 1.44

[martin-fleck-at]

@jfaltermeier Very interesting problem indeed! I also had a quick look at it and at your proposed solution but I believe the problem runs a bit deeper. So very simplified the issue is that we send two related events (changing a file and closing a file) to two different proxies (Documents and EditorAndDocuments). While messages for a single proxy are ensured to arrive in the same order (since they are using the same channel and RpcProtocol), messages to two different proxies are using two different channels and there is no guarantee that the messages arrive in the same order. @tortmayr Do you maybe know if that is always the case?

While in your case, the initialization of one of the proxies seems to create a delay that makes this problem very visible through the error, simply making the initialization faster will not fix the underlying issue and only ensure that it happens less likely.

I see that in the Documents and EditorsAndDocuments case, we followed the same structure as VS Code (i.e. two proxies) even though they are reacting to related events. I'm not sure if the same problem can also occur in VS Code but to me it seems that the cleanest solution would be to sync the events/messages somehow. From the top of my head, I'd say the easiest solution is to only use a single proxy but I'm not sure how that impacts performance.