From 1a2cc44fdbe68aa8c0c6e63c0e4668b095673ac5 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Mon, 2 Dec 2019 14:57:38 +0100 Subject: [PATCH] GH-6499: Explicitly close the socket `onStop` in electron Otherwise, the channels will be closed with a `checkAliveTimeout` delay in the electron application, when refreshing the browser window. Closes: #6499 Signed-off-by: Akos Kitta --- .../src/generator/frontend-generator.ts | 2 +- .../common/messaging/web-socket-channel.ts | 4 ++ .../electron-messaging-frontend-module.ts | 26 ++++++++++ .../electron-ws-connection-provider.ts | 47 +++++++++++++++++++ .../node/messaging/messaging-contribution.ts | 6 ++- .../search-in-workspace-frontend-module.ts | 4 +- .../common/search-in-workspace-interface.ts | 1 + .../search-in-workspace-backend-module.ts | 16 +++---- 8 files changed, 93 insertions(+), 13 deletions(-) create mode 100644 packages/core/src/electron-browser/messaging/electron-messaging-frontend-module.ts create mode 100644 packages/core/src/electron-browser/messaging/electron-ws-connection-provider.ts diff --git a/dev-packages/application-manager/src/generator/frontend-generator.ts b/dev-packages/application-manager/src/generator/frontend-generator.ts index 954f2bf07a02c..07bb216401530 100644 --- a/dev-packages/application-manager/src/generator/frontend-generator.ts +++ b/dev-packages/application-manager/src/generator/frontend-generator.ts @@ -69,7 +69,7 @@ require('reflect-metadata'); const { Container } = require('inversify'); const { FrontendApplication } = require('@theia/core/lib/browser'); const { frontendApplicationModule } = require('@theia/core/lib/browser/frontend-application-module'); -const { messagingFrontendModule } = require('@theia/core/lib/browser/messaging/messaging-frontend-module'); +const { messagingFrontendModule } = require('@theia/core/lib/${this.pck.isBrowser() ? 'browser/messaging/messaging-frontend-module' : 'electron-browser/messaging/electron-messaging-frontend-module'}'); const { loggerFrontendModule } = require('@theia/core/lib/browser/logger-frontend-module'); const { ThemeService } = require('@theia/core/lib/browser/theming'); const { FrontendApplicationConfigProvider } = require('@theia/core/lib/browser/frontend-application-config-provider'); diff --git a/packages/core/src/common/messaging/web-socket-channel.ts b/packages/core/src/common/messaging/web-socket-channel.ts index cc581db798922..5fa7d3a3c2997 100644 --- a/packages/core/src/common/messaging/web-socket-channel.ts +++ b/packages/core/src/common/messaging/web-socket-channel.ts @@ -79,6 +79,10 @@ export class WebSocketChannel implements IWebSocket { } close(code: number = 1000, reason: string = ''): void { + if (this.closing) { + // Do not try to close the channel if it is already closing. + return; + } this.checkNotDisposed(); this.doSend(JSON.stringify({ kind: 'close', diff --git a/packages/core/src/electron-browser/messaging/electron-messaging-frontend-module.ts b/packages/core/src/electron-browser/messaging/electron-messaging-frontend-module.ts new file mode 100644 index 0000000000000..6f8db65b589db --- /dev/null +++ b/packages/core/src/electron-browser/messaging/electron-messaging-frontend-module.ts @@ -0,0 +1,26 @@ +/******************************************************************************** + * Copyright (C) 2019 TypeFox 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 { ContainerModule } from 'inversify'; +import { FrontendApplicationContribution } from '../../browser/frontend-application'; +import { WebSocketConnectionProvider } from '../../browser/messaging/ws-connection-provider'; +import { ElectronWebSocketConnectionProvider } from './electron-ws-connection-provider'; + +export const messagingFrontendModule = new ContainerModule(bind => { + bind(ElectronWebSocketConnectionProvider).toSelf().inSingletonScope(); + bind(FrontendApplicationContribution).toService(ElectronWebSocketConnectionProvider); + bind(WebSocketConnectionProvider).toService(ElectronWebSocketConnectionProvider); +}); diff --git a/packages/core/src/electron-browser/messaging/electron-ws-connection-provider.ts b/packages/core/src/electron-browser/messaging/electron-ws-connection-provider.ts new file mode 100644 index 0000000000000..2bba1570874c3 --- /dev/null +++ b/packages/core/src/electron-browser/messaging/electron-ws-connection-provider.ts @@ -0,0 +1,47 @@ +/******************************************************************************** + * Copyright (C) 2019 TypeFox 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 } from 'inversify'; +import { WebSocketChannel } from '../../common/messaging/web-socket-channel'; +import { WebSocketConnectionProvider, WebSocketOptions } from '../../browser/messaging/ws-connection-provider'; +import { FrontendApplicationContribution } from '../../browser/frontend-application'; + +@injectable() +export class ElectronWebSocketConnectionProvider extends WebSocketConnectionProvider implements FrontendApplicationContribution { + + /** + * Do not try to reconnect when the frontend application is stopping. The browser is navigating away from this page. + */ + protected stopping = false; + + onStop(): void { + this.stopping = true; + // Close the websocket connection `onStop`. Otherwise, the channels will be closed with 30 sec (`MessagingContribution#checkAliveTimeout`) delay. + // https://github.com/eclipse-theia/theia/issues/6499 + for (const channel of [...this.channels.values()]) { + // `1001` indicates that an endpoint is "going away", such as a server going down or a browser having navigated away from a page. + // But we cannot use `1001`: https://github.com/TypeFox/vscode-ws-jsonrpc/issues/15 + channel.close(1000, 'The frontend is "going away"...'); + } + } + + openChannel(path: string, handler: (channel: WebSocketChannel) => void, options?: WebSocketOptions): void { + if (!this.stopping) { + super.openChannel(path, handler, options); + } + } + +} diff --git a/packages/core/src/node/messaging/messaging-contribution.ts b/packages/core/src/node/messaging/messaging-contribution.ts index 1ab7cbe683f0d..b85167fd3c8c4 100644 --- a/packages/core/src/node/messaging/messaging-contribution.ts +++ b/packages/core/src/node/messaging/messaging-contribution.ts @@ -127,8 +127,12 @@ export class MessagingContribution implements BackendApplicationContribution, Me const channel = this.createChannel(id, socket); if (channelHandlers.route(path, channel)) { channel.ready(); + console.debug(`Opening channel for service path '${path}'. [ID: ${id}]`); channels.set(id, channel); - channel.onClose(() => channels.delete(id)); + channel.onClose(() => { + console.debug(`Closing channel on service path '${path}'. [ID: ${id}]`); + channels.delete(id); + }); } else { console.error('Cannot find a service for the path: ' + path); } diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-module.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-module.ts index 8faa063529446..f8c4f51f9324a 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-module.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-module.ts @@ -18,7 +18,7 @@ import '../../src/browser/styles/index.css'; import { ContainerModule, interfaces } from 'inversify'; import { SearchInWorkspaceService, SearchInWorkspaceClientImpl } from './search-in-workspace-service'; -import { SearchInWorkspaceServer } from '../common/search-in-workspace-interface'; +import { SearchInWorkspaceServer, SIW_WS_PATH } from '../common/search-in-workspace-interface'; import { WebSocketConnectionProvider, WidgetFactory, createTreeContainer, TreeWidget, bindViewContribution, FrontendApplicationContribution } from '@theia/core/lib/browser'; import { ResourceResolver } from '@theia/core'; import { SearchInWorkspaceWidget } from './search-in-workspace-widget'; @@ -51,7 +51,7 @@ export default new ContainerModule(bind => { // The object to call methods on the backend. bind(SearchInWorkspaceServer).toDynamicValue(ctx => { const client = ctx.container.get(SearchInWorkspaceClientImpl); - return WebSocketConnectionProvider.createProxy(ctx.container, '/search-in-workspace', client); + return WebSocketConnectionProvider.createProxy(ctx.container, SIW_WS_PATH, client); }).inSingletonScope(); bind(InMemoryTextResourceResolver).toSelf().inSingletonScope(); diff --git a/packages/search-in-workspace/src/common/search-in-workspace-interface.ts b/packages/search-in-workspace/src/common/search-in-workspace-interface.ts index 0a0307f6e806a..2a07a130d19b3 100644 --- a/packages/search-in-workspace/src/common/search-in-workspace-interface.ts +++ b/packages/search-in-workspace/src/common/search-in-workspace-interface.ts @@ -116,6 +116,7 @@ export interface SearchInWorkspaceClient { onDone(searchId: number, error?: string): void; } +export const SIW_WS_PATH = '/services/search-in-workspace'; export const SearchInWorkspaceServer = Symbol('SearchInWorkspaceServer'); export interface SearchInWorkspaceServer extends JsonRpcServer { /** diff --git a/packages/search-in-workspace/src/node/search-in-workspace-backend-module.ts b/packages/search-in-workspace/src/node/search-in-workspace-backend-module.ts index 5bea019d7fed5..1898ad34feb88 100644 --- a/packages/search-in-workspace/src/node/search-in-workspace-backend-module.ts +++ b/packages/search-in-workspace/src/node/search-in-workspace-backend-module.ts @@ -16,20 +16,18 @@ import { ContainerModule } from 'inversify'; import { ConnectionHandler, JsonRpcConnectionHandler } from '@theia/core/lib/common'; -import { SearchInWorkspaceServer, SearchInWorkspaceClient } from '../common/search-in-workspace-interface'; +import { SearchInWorkspaceServer, SearchInWorkspaceClient, SIW_WS_PATH } from '../common/search-in-workspace-interface'; import { RipgrepSearchInWorkspaceServer, RgPath } from './ripgrep-search-in-workspace-server'; import { rgPath } from 'vscode-ripgrep'; export default new ContainerModule(bind => { bind(SearchInWorkspaceServer).to(RipgrepSearchInWorkspaceServer); bind(ConnectionHandler).toDynamicValue(ctx => - new JsonRpcConnectionHandler - ('/search-in-workspace', client => { - const server = ctx.container.get(SearchInWorkspaceServer); - server.setClient(client); - client.onDidCloseConnection(() => server.dispose()); - return server; - }) - ); + new JsonRpcConnectionHandler(SIW_WS_PATH, client => { + const server = ctx.container.get(SearchInWorkspaceServer); + server.setClient(client); + client.onDidCloseConnection(() => server.dispose()); + return server; + })); bind(RgPath).toConstantValue(rgPath); });