Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[plugin] support multiple windows per a backend #4521

Merged
merged 1 commit into from
Mar 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

Breaking changes:
- [editor] computation of resource context keys moved to core [#4531](https://github.com/theia-ide/theia/pull/4531)
- [plugin] support multiple windows per a backend [#4509](https://github.com/theia-ide/theia/issues/4509)
- Some plugin bindings are scoped per a connection now. Clients, who contribute/rebind these bindings, will need to scope them per a connection as well.

## v0.4.0
- [application-manager] added support for pre-load HTML templates
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,18 @@

import { HostedInstanceManager, ElectronNodeHostedPluginRunner } from '../node/hosted-instance-manager';
import { interfaces } from 'inversify';
import { ConnectionContainerModule } from '@theia/core/lib/node/messaging/connection-container-module';
import { bindCommonHostedBackend } from '../node/plugin-ext-hosted-backend-module';
import { PluginScanner } from '../../common/plugin-protocol';
import { TheiaPluginScannerElectron } from './scanner-theia-electron';

const hostedBackendConnectionModule = ConnectionContainerModule.create(({ bind }) => {
bind(HostedInstanceManager).to(ElectronNodeHostedPluginRunner);
});

export function bindElectronBackend(bind: interfaces.Bind): void {
bindCommonHostedBackend(bind);
bind(ConnectionContainerModule).toConstantValue(hostedBackendConnectionModule);

bind(HostedInstanceManager).to(ElectronNodeHostedPluginRunner);
bind(PluginScanner).to(TheiaPluginScannerElectron).inSingletonScope();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/********************************************************************************
* Copyright (C) 2019 RedHat and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { injectable, inject } from 'inversify';
import { ILogger } from '@theia/core';
import { PluginDeployerHandler, PluginDeployerEntry, PluginMetadata } from '../../common/plugin-protocol';
import { HostedPluginReader } from './plugin-reader';

@injectable()
export class HostedPluginDeployerHandler implements PluginDeployerHandler {

@inject(ILogger)
protected readonly logger: ILogger;

@inject(HostedPluginReader)
private readonly reader: HostedPluginReader;

/**
* Managed plugin metadata backend entries.
*/
private readonly currentBackendPluginsMetadata: PluginMetadata[] = [];

/**
* Managed plugin metadata frontend entries.
*/
private readonly currentFrontendPluginsMetadata: PluginMetadata[] = [];

getDeployedFrontendMetadata(): PluginMetadata[] {
return this.currentFrontendPluginsMetadata;
}

getDeployedBackendMetadata(): PluginMetadata[] {
return this.currentBackendPluginsMetadata;
}

async deployFrontendPlugins(frontendPlugins: PluginDeployerEntry[]): Promise<void> {
for (const plugin of frontendPlugins) {
const metadata = await this.reader.getPluginMetadata(plugin.path());
if (metadata) {
this.currentFrontendPluginsMetadata.push(metadata);
this.logger.info(`Deploying frontend plugin "${metadata.model.name}@${metadata.model.version}" from "${metadata.model.entryPoint.frontend || plugin.path()}"`);
}
}
}

async deployBackendPlugins(backendPlugins: PluginDeployerEntry[]): Promise<void> {
for (const plugin of backendPlugins) {
const metadata = await this.reader.getPluginMetadata(plugin.path());
if (metadata) {
this.currentBackendPluginsMetadata.push(metadata);
this.logger.info(`Deploying backend plugin "${metadata.model.name}@${metadata.model.version}" from "${metadata.model.entryPoint.backend || plugin.path()}"`);
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import { interfaces } from 'inversify';
import { bindContributionProvider } from '@theia/core/lib/common/contribution-provider';
import { CliContribution } from '@theia/core/lib/node/cli';
import { ConnectionContainerModule } from '@theia/core/lib/node/messaging/connection-container-module';
import { HostedInstanceManager, NodeHostedPluginRunner } from './hosted-instance-manager';
import { HostedPluginUriPostProcessorSymbolName } from './hosted-plugin-uri-postprocessor';
import { ConnectionHandler, JsonRpcConnectionHandler } from '@theia/core/lib/common/messaging';
import { BackendApplicationContribution } from '@theia/core/lib/node/backend-application';
import { MetadataScanner } from './metadata-scanner';
import { HostedPluginServerImpl } from './plugin-service';
Expand All @@ -32,44 +32,49 @@ import { GrammarsReader } from './scanners/grammars-reader';
import { HostedPluginProcess } from './hosted-plugin-process';
import { ExtPluginApiProvider } from '../../common/plugin-ext-api-contribution';
import { HostedPluginCliContribution } from './hosted-plugin-cli-contribution';
import { HostedPluginDeployerHandler } from './hosted-plugin-deployer-handler';

export function bindCommonHostedBackend(bind: interfaces.Bind): void {
bind(HostedPluginCliContribution).toSelf().inSingletonScope();
bind(CliContribution).toService(HostedPluginCliContribution);
const commonHostedConnectionModule = ConnectionContainerModule.create(({ bind, bindBackendService }) => {
bind(HostedPluginProcess).toSelf().inSingletonScope();
bind(HostedPluginSupport).toSelf().inSingletonScope();

bind(HostedPluginReader).toSelf().inSingletonScope();
bind(HostedPluginsManagerImpl).toSelf().inSingletonScope();
bind(HostedPluginsManager).toService(HostedPluginsManagerImpl);

bindContributionProvider(bind, Symbol.for(ExtPluginApiProvider));
bind(HostedPluginServerImpl).toSelf().inSingletonScope();
bind(HostedPluginServer).toService(HostedPluginServerImpl);
bind(PluginDeployerHandler).toService(HostedPluginServerImpl);

bind(HostedPluginSupport).toSelf().inSingletonScope();
bind(MetadataScanner).toSelf().inSingletonScope();
bind(HostedPluginsManager).to(HostedPluginsManagerImpl).inSingletonScope();
bindBackendService<HostedPluginServer, HostedPluginClient>(hostedServicePath, HostedPluginServer, (server, client) => {
server.setClient(client);
client.onDidCloseConnection(() => server.dispose());
return server;
});
});

bind(HostedPluginProcess).toSelf().inSingletonScope();
export function bindCommonHostedBackend(bind: interfaces.Bind): void {
bind(HostedPluginCliContribution).toSelf().inSingletonScope();
bind(CliContribution).toService(HostedPluginCliContribution);

bind(MetadataScanner).toSelf().inSingletonScope();
bind(HostedPluginReader).toSelf().inSingletonScope();
bind(BackendApplicationContribution).toService(HostedPluginReader);

bind(ConnectionHandler).toDynamicValue(ctx =>
new JsonRpcConnectionHandler<HostedPluginClient>(hostedServicePath, client => {
const server = ctx.container.get<HostedPluginServer>(HostedPluginServer);
server.setClient(client);
// FIXME: handle multiple remote connections
/*
client.onDidCloseConnection(() => server.dispose());*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that server.dispose() would be useful in the new case (as we need to inform somehow that client is not connected if instances are not automatically destroyed ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be auto called when connection is closed, i wonder why it does not, will check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, i am wrong, it is not auto disposed, this line is needed, checking again and pushing

return server;
})
).inSingletonScope();
bind(HostedPluginDeployerHandler).toSelf().inSingletonScope();
bind(PluginDeployerHandler).toService(HostedPluginDeployerHandler);

bind(GrammarsReader).toSelf().inSingletonScope();

bind(ConnectionContainerModule).toConstantValue(commonHostedConnectionModule);
}

const hostedBackendConnectionModule = ConnectionContainerModule.create(({ bind }) => {
bindContributionProvider(bind, Symbol.for(HostedPluginUriPostProcessorSymbolName));
bind(HostedInstanceManager).to(NodeHostedPluginRunner).inSingletonScope();
});

export function bindHostedBackend(bind: interfaces.Bind): void {
bindCommonHostedBackend(bind);
bind(ConnectionContainerModule).toConstantValue(hostedBackendConnectionModule);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello, simple question, is that ConnectionContainerModule is handling the destroy of the instances associated to the scope ? (closing the browser tab is destroying the instances of this scope ) ?

Copy link
Member Author

@akosyakov akosyakov Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it should, instances only exist within connection scope and can access each other (be injected)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

closing the browser tab is destroying the instances of this scope

@svenefftinge it was my assumption. It could be good to implement, otherwise it could be hard to clean up without explicitly binding backend service.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I assume we may have leaks on many items if instances are not destroyed/not notified ?


bind(HostedInstanceManager).to(NodeHostedPluginRunner).inSingletonScope();
bind(PluginScanner).to(TheiaPluginScanner).inSingletonScope();
bindContributionProvider(bind, Symbol.for(HostedPluginUriPostProcessorSymbolName));
bindContributionProvider(bind, Symbol.for(ExtPluginApiProvider));
}
55 changes: 14 additions & 41 deletions packages/plugin-ext/src/hosted/node/plugin-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { injectable, inject, named } from 'inversify';
import { HostedPluginServer, HostedPluginClient, PluginMetadata, PluginDeployerEntry, DebugConfiguration, PluginDeployerHandler } from '../../common/plugin-protocol';
import { HostedPluginServer, HostedPluginClient, PluginMetadata, DebugConfiguration } from '../../common/plugin-protocol';
import { HostedPluginReader } from './plugin-reader';
import { HostedInstanceManager } from './hosted-instance-manager';
import { HostedPluginSupport } from './hosted-plugin';
Expand All @@ -23,28 +23,22 @@ import URI from '@theia/core/lib/common/uri';
import { ILogger } from '@theia/core';
import { ContributionProvider } from '@theia/core';
import { ExtPluginApiProvider, ExtPluginApi } from '../../common/plugin-ext-api-contribution';
import { HostedPluginDeployerHandler } from './hosted-plugin-deployer-handler';

@injectable()
export class HostedPluginServerImpl implements HostedPluginServer, PluginDeployerHandler {
export class HostedPluginServerImpl implements HostedPluginServer {
@inject(ILogger)
protected readonly logger: ILogger;
@inject(HostedPluginsManager)
protected readonly hostedPluginsManager: HostedPluginsManager;

@inject(HostedPluginDeployerHandler)
protected readonly deployerHandler: HostedPluginDeployerHandler;

@inject(ContributionProvider)
@named(Symbol.for(ExtPluginApiProvider))
protected readonly extPluginAPIContributions: ContributionProvider<ExtPluginApiProvider>;

/**
* Managed plugin metadata backend entries.
*/
private currentBackendPluginsMetadata: PluginMetadata[] = [];

/**
* Managed plugin metadata frontend entries.
*/
private currentFrontendPluginsMetadata: PluginMetadata[] = [];

constructor(
@inject(HostedPluginReader) private readonly reader: HostedPluginReader,
@inject(HostedPluginSupport) private readonly hostedPlugin: HostedPluginSupport,
Expand All @@ -66,13 +60,17 @@ export class HostedPluginServerImpl implements HostedPluginServer, PluginDeploye
}

getDeployedFrontendMetadata(): Promise<PluginMetadata[]> {
return Promise.resolve(this.currentFrontendPluginsMetadata);
return Promise.resolve(this.deployerHandler.getDeployedFrontendMetadata());
}

async getDeployedMetadata(): Promise<PluginMetadata[]> {
const backendMetadata = this.deployerHandler.getDeployedBackendMetadata();
if (backendMetadata.length > 0) {
this.hostedPlugin.runPluginServer();
}
const allMetadata: PluginMetadata[] = [];
allMetadata.push(...this.currentFrontendPluginsMetadata);
allMetadata.push(...this.currentBackendPluginsMetadata);
allMetadata.push(...this.deployerHandler.getDeployedFrontendMetadata());
allMetadata.push(...backendMetadata);

// ask remote as well
const extraBackendPluginsMetadata = await this.hostedPlugin.getExtraPluginMetadata();
Expand All @@ -81,33 +79,8 @@ export class HostedPluginServerImpl implements HostedPluginServer, PluginDeploye
return allMetadata;
}

// need to run a new node instance with plugin-host for all plugins
async deployFrontendPlugins(frontendPlugins: PluginDeployerEntry[]): Promise<void> {
for (const plugin of frontendPlugins) {
const metadata = await this.reader.getPluginMetadata(plugin.path());
if (metadata) {
this.currentFrontendPluginsMetadata.push(metadata);
this.logger.info(`Deploying frontend plugin "${metadata.model.name}@${metadata.model.version}" from "${metadata.model.entryPoint.frontend || plugin.path()}"`);
}
}
}

getDeployedBackendMetadata(): Promise<PluginMetadata[]> {
return Promise.resolve(this.currentBackendPluginsMetadata);
}

// need to run a new node instance with plugin-host for all plugins
async deployBackendPlugins(backendPlugins: PluginDeployerEntry[]): Promise<void> {
if (backendPlugins.length > 0) {
this.hostedPlugin.runPluginServer();
}
for (const plugin of backendPlugins) {
const metadata = await this.reader.getPluginMetadata(plugin.path());
if (metadata) {
this.currentBackendPluginsMetadata.push(metadata);
this.logger.info(`Deploying backend plugin "${metadata.model.name}@${metadata.model.version}" from "${metadata.model.entryPoint.backend || plugin.path()}"`);
}
}
return Promise.resolve(this.deployerHandler.getDeployedBackendMetadata());
}

onMessage(message: string): Promise<void> {
Expand Down