Inconsistent behavior in HostedPluginSupport.activateByEvent(activationEvent: string)

[jcortell68]

Bug Description:

HostedPluginSupport.activateByEvent() is used by Theia code to support VS Code extension activation events. The event is a string, e.g., “onLanguage:jsonc”. If a VS Code extension declares that string in its activationEvents[] element in package.json, then the call HostedPluginSupport.activateByEvent('onLanguage:jsonc') by a Theia extension (i.e., “the app”) should cause that plugin to be activated–have its main module loaded and its activate() function called.

Furthermore, that method is an async command and the Promise it returns resolves only after all the activated plugins’ activate methods have completed. So if in the Theia extension you call that with await, that of course means the call blocks until all the activated plugins have completed running their activation code.

And this works exactly as described…except if the call is made prior to the plugin-host process being forked. In that case, the HostedPluginSupport implementation sees there is no plugin-host yet, and proceeds to just record the activation event in a sort of ‘pending’ list and tends to it when the plugin-host comes online.

And this might work well for all the VS Code activationEvents currently supported by Theia. I imagine most such activation events are not timing critical, meaning that the plugins being activated 2-20 seconds after the event actually happened doesn’t necessarily break functionality.

But that is not necessarily the case in general. I.e., as a producer of a Theia product with custom plugin API namespaces, I might introduce an activation event that is timing critical. I.e., I need HostedPluginSupport.activateByEvent() to behave the same way in all cases–before and after the plugin-host is online. In other words, if the plugin host is not yet online, I want the Promise to not resolve until it has come online and all relevant plugins have had their activation methods executed and completed. Obviously, to avoid breaking functionality, this should not be the default behavior but optional behavior I can request via a new argument.

I’ve created a Hello World Theia example that demonstrates the problem. The app has a custom plugin API namespace. The app has one purpose: to say hello world via a message box. By default, it will say it in English. But the app provides a custom API that allows a plugin to replace the message that will be displayed. The plugin shouldn’t be activated if the user doesn’t end up asking the app to say hello. And thus the app introduces the activation event onSayHello. The github repo for the app also includes a plugin which declares that activation event in its package.json. What it will do during activation is use the custom API to call back into the app with an alternate message string–one that’s in Spanish. It will make that API call with await to ensure the registration happens before it returns control back to the app (i.e., before it returns from the activate function).

The net effect of all this is that the user, by having that plugin, can rest assured he'll receive the message in Spanish.

And this works exactly as expected…as long as the user triggers the Say Hello menu item after the plugin host has come online. But that can take some time. On my machine, it takes anywhere from six seconds to 18 seconds. That’s a wide range, and I’m not sure why it’s that wide, but it’s a non-trivial amount of time in many cases. And this is a small Theia app. In theory, a more substantial app might take even longer. But the menu item is available to the user quite quickly after launch. And so you have this window in which the user can initiate the command and end up with an unexpected result–a message in English.

As the developer of the Theia app, I would like to ensure that doesn’t happen by asking HostedPluginSupport.activateByEvent() to wait for the plugin-host (if it’s not already running). Of course, I’ll also need to document the plugin API so developers know to use await when calling the setHelloMessage() API function from the activate function, to ensure the user will see his message in all cases.

BTW, this is a fabricated situation for the purpose of demonstrating the problem. Obviously, this approach is not the right one for product internationalization. I ran into this deficiency not by looking for it but by spending hours debugging why sometimes my app didn’t behave as I expected it to–and, btw, not realizing it was related to the timing of the plugin-host startup. Fun! (not)

Steps to Reproduce:

git clone -b activation_event_gap https://github.com/jcortell68/theiasandbox
cd theiasandbox
yarn
yarn start:electron

1. Look at stdout and wait until you see messages from the plugin-host node process. They usually look like this

   root INFO [hosted-plugin: <pid>] PLUGIN_HOST(<pid>) some message

2. Invoke the menu item Help > Say Hello
3. Notice you get a greeting in Spanish. This is the expected behavior
4. Restart the app (ctrl+r)
5. As soon as the menu bar is available, trigger the Say Hello menu item.
6. Assuming you did it fast enough (before the plugin-host came up), you should see the message in English. This is unwanted behavior,.

[jcortell68]

BTW, in case someone is wondering...how is this situation not a problem for command contributions from VS Code extensions??? Well, in that case, Theia doesn't just call HostedPluginSupport.activateByEvent(onCommand:my.command). It does so (using await of course), but then it proceeds to logic that waits until my.command has a handler registered to it. The thinking there of course is that if a VS Code extension declares a command contribution in its package, it will be the one registering a handler for it via its activate() function. So Theia waits for the registration to happen and doesn't actually try to invoke the command until the registration happens (or 30 seconds goes by).

This approach would probably need to remain even if the problem described here is fixed. The reason for that is that VS Code extension developers probably don't use await when calling the function that registers the command handler. So even if we ensure all this synchronicity of activation, the asynchronous way in which plugins register command handlers would make it a moot point. But that doesn't mean new product-specific API can't tell developers to use await to avoid the type of latency problem described here, when and only when it matters of course. Also keep in mind that the command handler situation is somewhat unique because the activation event is qualified with a command ID. Thus, it's easy for Theia to wait for that specific command handler to be registered. The same cannot be said of my fabricated scenario, and it can't be said for my real-world scenario, either.

[jfaltermeier]

Hi, I am having issues reproducing the bug. I have tried on Ubuntu and in a Windows VM and the start up seems too fast in both cases for me. I tried to add some delays in the code to make it slower for debugging, but this always delayed the message from showing instead of showing it in English.

What I can see in the hosted-plugin.ts activateByEvent implementation is that it will return immediately when there are no managers. If there are managers, then the activation will be delegated to each one of them and the Promise finishes when all of them are finished. activateByEvent requests are stored, so that they can be passed to a manager's start method, if the manager gets started after activateByEvent was called already.

Is the manager list still empty in your reproducer case? Does calling didStart on the plugin support help before activation?

await this.hostedPluginSupport.didStart;
await this.hostedPluginSupport.activateByEvent('onSayHello');

[jcortell68]

@jfaltermeier I update my theiasandbox repo to that branch, rebuilt, and I can still reproduce the problem. See below. Either I'd beat you in a High Noon style pistol face-off, or my Windows machine is much slower than your environments. I'd bet on the latter.

[jcortell68]

Does calling didStart on the plugin support help before activation?

Ooooooh. I did not see that. That might be a super simple resolution to a very lengthy (to explain) problem. I'll let you know

[jcortell68]

Problem solved. That did indeed do the trick. I'll close the ticket. Thanks, @jfaltermeier

await this.hostedPluginSupport.didStart;
await this.hostedPluginSupport.activateByEvent('onSayHello');