From 79fc062ea35c14630df12249b3276d3a32868c71 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 15:32:29 -0500 Subject: [PATCH 01/14] Make proxies decide how to handle disconnects --- packages/protocol/src/browser/client.ts | 16 ++++-------- .../src/browser/modules/child_process.ts | 10 ++++++++ packages/protocol/src/browser/modules/fs.ts | 9 +++++++ packages/protocol/src/browser/modules/net.ts | 17 ++++++++++++- .../protocol/src/browser/modules/node-pty.ts | 5 ++++ .../protocol/src/browser/modules/spdlog.ts | 4 +++ .../protocol/src/browser/modules/stream.ts | 25 +++++++++++++++++++ packages/protocol/src/common/proxy.ts | 3 +++ 8 files changed, 77 insertions(+), 12 deletions(-) diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 470c862afe34..854fec5054d1 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -134,14 +134,8 @@ export class Client { message.setResponse(stringify(error)); this.failEmitter.emit(message); - this.eventEmitter.emit({ event: "exit", args: [1] }); - this.eventEmitter.emit({ event: "close", args: [] }); - try { - this.eventEmitter.emit({ event: "error", args: [error] }); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } - this.eventEmitter.emit({ event: "done", args: [true] }); + this.eventEmitter.emit({ event: "disconnected", args: [error] }); + this.eventEmitter.emit({ event: "done", args: [] }); }; connection.onDown(() => handleDisconnect()); @@ -450,12 +444,12 @@ export class Client { callbacks: new Map(), }); - instance.onDone((disconnected: boolean) => { + instance.onDone(() => { const log = (): void => { logger.trace(() => [ typeof proxyId === "number" ? "disposed proxy" : "disposed proxy callbacks", field("proxyId", proxyId), - field("disconnected", disconnected), + field("disconnected", this.disconnected), field("callbacks", Array.from(this.proxies.values()).reduce((count, proxy) => count + proxy.callbacks.size, 0)), field("success listeners", this.successEmitter.counts), field("fail listeners", this.failEmitter.counts), @@ -471,7 +465,7 @@ export class Client { this.eventEmitter.dispose(proxyId); log(); }; - if (!disconnected) { + if (!this.disconnected) { instance.dispose().then(dispose).catch(dispose); } else { dispose(); diff --git a/packages/protocol/src/browser/modules/child_process.ts b/packages/protocol/src/browser/modules/child_process.ts index de13e171866d..17e90b48024e 100644 --- a/packages/protocol/src/browser/modules/child_process.ts +++ b/packages/protocol/src/browser/modules/child_process.ts @@ -87,6 +87,16 @@ export class ChildProcess extends ClientProxy implements cp.C return true; // Always true since we can't get this synchronously. } + + protected handleDisconnect(error: Error): void { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.emit("exit", 1); + this.emit("close"); + } } export class ChildProcessModule { diff --git a/packages/protocol/src/browser/modules/fs.ts b/packages/protocol/src/browser/modules/fs.ts index bb23c6b06f5c..a25ee2d0af60 100644 --- a/packages/protocol/src/browser/modules/fs.ts +++ b/packages/protocol/src/browser/modules/fs.ts @@ -41,6 +41,15 @@ class Watcher extends ClientProxy implements fs.FSWatcher { public close(): void { this.proxy.close(); } + + protected handleDisconnect(error: Error): void { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.emit("close"); + } } class WriteStream extends Writable implements fs.WriteStream { diff --git a/packages/protocol/src/browser/modules/net.ts b/packages/protocol/src/browser/modules/net.ts index 723bb1b11512..bda36c94adc2 100644 --- a/packages/protocol/src/browser/modules/net.ts +++ b/packages/protocol/src/browser/modules/net.ts @@ -126,6 +126,7 @@ export class Socket extends Duplex implements net.Socket { } export class Server extends ClientProxy implements net.Server { + private socketId = 0; private readonly sockets = new Map(); private _listening: boolean = false; @@ -133,7 +134,12 @@ export class Server extends ClientProxy implements net.Server { super(proxyPromise); this.proxy.onConnection((socketProxy) => { - this.emit("connection", new Socket(socketProxy)); + const socket = new Socket(socketProxy); + const socketId = this.socketId++; + this.sockets.set(socketId, socket); + socket.on("error", () => this.sockets.delete(socketId)) + socket.on("close", () => this.sockets.delete(socketId)) + this.emit("connection", socket); }); this.on("listening", () => this._listening = true); @@ -200,6 +206,15 @@ export class Server extends ClientProxy implements net.Server { public getConnections(cb: (error: Error | null, count: number) => void): void { cb(null, this.sockets.size); } + + protected handleDisconnect(error: Error): void { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.emit("close"); + } } type NodeNet = typeof net; diff --git a/packages/protocol/src/browser/modules/node-pty.ts b/packages/protocol/src/browser/modules/node-pty.ts index 7c96859fd97b..48920aa91b57 100644 --- a/packages/protocol/src/browser/modules/node-pty.ts +++ b/packages/protocol/src/browser/modules/node-pty.ts @@ -32,6 +32,11 @@ export class NodePtyProcess extends ClientProxy implements public kill(signal?: string): void { this.proxy.kill(signal); } + + protected handleDisconnect(): void { + this._process += " (disconnected)"; + this.emit("data", "\r\n\nLost connection..."); + } } type NodePty = typeof pty; diff --git a/packages/protocol/src/browser/modules/spdlog.ts b/packages/protocol/src/browser/modules/spdlog.ts index 9d10fbab2f19..225a58089c2e 100644 --- a/packages/protocol/src/browser/modules/spdlog.ts +++ b/packages/protocol/src/browser/modules/spdlog.ts @@ -13,6 +13,10 @@ class RotatingLogger extends ClientProxy implements spdlog. public async clearFormatters (): Promise { this.proxy.clearFormatters(); } public async flush (): Promise { this.proxy.flush(); } public async drop (): Promise { this.proxy.drop(); } + + protected handleDisconnect(): void { + // TODO: reconnect. + } } export class SpdlogModule { diff --git a/packages/protocol/src/browser/modules/stream.ts b/packages/protocol/src/browser/modules/stream.ts index f5db4644503a..eee58e013147 100644 --- a/packages/protocol/src/browser/modules/stream.ts +++ b/packages/protocol/src/browser/modules/stream.ts @@ -81,6 +81,16 @@ export class Writable extends ClientPro } }); } + + protected handleDisconnect(error: Error): void { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.emit("close"); + this.emit("finish"); + } } export class Readable extends ClientProxy implements stream.Readable { @@ -154,6 +164,16 @@ export class Readable extends ClientP return this; } + + protected handleDisconnect(error: Error): void { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.emit("close"); + this.emit("end"); + } } export class Duplex extends Writable implements stream.Duplex, stream.Readable { @@ -230,4 +250,9 @@ export class Duplex extends Writable imp return this; } + + protected handleDisconnect(error: Error): void { + super.handleDisconnect(error); + this.emit("end"); + } } diff --git a/packages/protocol/src/common/proxy.ts b/packages/protocol/src/common/proxy.ts index 6558ea2a41f8..13c7044c9a69 100644 --- a/packages/protocol/src/common/proxy.ts +++ b/packages/protocol/src/common/proxy.ts @@ -42,8 +42,11 @@ export abstract class ClientProxy extends EventEmitter { this.proxy.onEvent((event, ...args): void => { this.emit(event, ...args); }); + this.on("disconnected", (error) => this.handleDisconnect(error)); } } + + protected abstract handleDisconnect(error: Error): void; } /** From 7b0c98495ded781b63880573e004694058dc4c2d Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 16:31:21 -0500 Subject: [PATCH 02/14] Connect to a new terminal instance on disconnect --- packages/protocol/src/browser/client.ts | 17 +++++++++--- .../protocol/src/browser/modules/node-pty.ts | 20 ++++++++++---- packages/protocol/src/common/proxy.ts | 27 +++++++++++++++---- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 854fec5054d1..58034eb08ba0 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -143,6 +143,7 @@ export class Client { clearTimeout(this.pingTimeout as any); this.pingTimeout = undefined; handleDisconnect(); + this.dispose(); }); connection.onUp(() => this.disconnected = false); @@ -158,6 +159,12 @@ export class Client { */ public dispose(): void { this.connection.close(); + this.proxies.clear(); + this.successEmitter.dispose(); + this.failEmitter.dispose(); + this.eventEmitter.dispose(); + this.initDataEmitter.dispose(); + this.sharedProcessActiveEmitter.dispose(); } public get initData(): Promise { @@ -167,9 +174,11 @@ export class Client { /** * Make a remote call for a proxy's method using proto. */ - private remoteCall(proxyId: number | Module, method: string, args: any[]): Promise { - if (this.disconnected) { - return Promise.reject(new Error("disconnected")); + private async remoteCall(proxyId: number | Module, method: string, args: any[]): Promise { + if (this.disconnected && typeof proxyId === "number") { + return Promise.reject( + new Error(`Unable to call "${method}" on proxy ${proxyId}: disconnected`), + ); } const message = new MethodMessage(); @@ -217,7 +226,7 @@ export class Client { // The server will send back a fail or success message when the method // has completed, so we listen for that based on the message's unique ID. - const promise = new Promise((resolve, reject): void => { + const promise = new Promise((resolve, reject): void => { const dispose = (): void => { d1.dispose(); d2.dispose(); diff --git a/packages/protocol/src/browser/modules/node-pty.ts b/packages/protocol/src/browser/modules/node-pty.ts index 48920aa91b57..997ba941cfb3 100644 --- a/packages/protocol/src/browser/modules/node-pty.ts +++ b/packages/protocol/src/browser/modules/node-pty.ts @@ -6,11 +6,20 @@ export class NodePtyProcess extends ClientProxy implements private _pid = -1; private _process = ""; - public constructor(proxyPromise: Promise) { - super(proxyPromise); + public constructor( + private readonly moduleProxy: NodePtyModuleProxy, + private readonly file: string, + private readonly args: string[] | string, + private readonly options: pty.IPtyForkOptions, + ) { + super(moduleProxy.spawn(file, args, options)); + this.on("process", (process) => this._process = process); + } + + protected initialize(proxyPromise: Promise) { + super.initialize(proxyPromise); this.proxy.getPid().then((pid) => this._pid = pid); this.proxy.getProcess().then((process) => this._process = process); - this.on("process", (process) => this._process = process); } public get pid(): number { @@ -35,7 +44,8 @@ export class NodePtyProcess extends ClientProxy implements protected handleDisconnect(): void { this._process += " (disconnected)"; - this.emit("data", "\r\n\nLost connection..."); + this.emit("data", "\r\n\nLost connection...\r\n\n"); + this.initialize(this.moduleProxy.spawn(this.file, this.args, this.options)); } } @@ -45,6 +55,6 @@ export class NodePtyModule implements NodePty { public constructor(private readonly proxy: NodePtyModuleProxy) {} public spawn = (file: string, args: string[] | string, options: pty.IPtyForkOptions): pty.IPty => { - return new NodePtyProcess(this.proxy.spawn(file, args, options)); + return new NodePtyProcess(this.proxy, file, args, options); } } diff --git a/packages/protocol/src/common/proxy.ts b/packages/protocol/src/common/proxy.ts index 13c7044c9a69..c91e295f51bd 100644 --- a/packages/protocol/src/common/proxy.ts +++ b/packages/protocol/src/common/proxy.ts @@ -29,20 +29,37 @@ const unpromisify = (proxyPromise: Promise): T => { * need a bunch of `then` calls everywhere. */ export abstract class ClientProxy extends EventEmitter { - protected readonly proxy: T; + private _proxy: T | undefined; /** * You can specify not to bind events in order to avoid emitting twice for * duplex streams. */ - public constructor(proxyPromise: Promise | T, bindEvents: boolean = true) { + public constructor( + proxyPromise: Promise | T, + private readonly bindEvents: boolean = true, + ) { super(); - this.proxy = isPromise(proxyPromise) ? unpromisify(proxyPromise) : proxyPromise; - if (bindEvents) { + this.initialize(proxyPromise); + if (this.bindEvents) { + this.on("disconnected", (error) => this.handleDisconnect(error)); + } + } + + protected get proxy(): T { + if (!this._proxy) { + throw new Error("not initialized"); + } + + return this._proxy; + } + + protected initialize(proxyPromise: Promise | T): void { + this._proxy = isPromise(proxyPromise) ? unpromisify(proxyPromise) : proxyPromise; + if (this.bindEvents) { this.proxy.onEvent((event, ...args): void => { this.emit(event, ...args); }); - this.on("disconnected", (error) => this.handleDisconnect(error)); } } From 95ef2ee4f98e51b51a6d765817516f6449f2a5e4 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 16:48:03 -0500 Subject: [PATCH 03/14] Use our retry for the watcher --- packages/vscode/src/workbench.ts | 2 ++ scripts/vscode.patch | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/vscode/src/workbench.ts b/packages/vscode/src/workbench.ts index 4e023ed149c0..73fc77e96f7c 100644 --- a/packages/vscode/src/workbench.ts +++ b/packages/vscode/src/workbench.ts @@ -30,6 +30,8 @@ import { ServiceCollection } from "vs/platform/instantiation/common/serviceColle import { URI } from "vs/base/common/uri"; export class Workbench { + public readonly retry = client.retry; + private readonly windowId = parseInt(new Date().toISOString().replace(/[-:.TZ]/g, ""), 10); private _serviceCollection: ServiceCollection | undefined; private _clipboardContextKey: RawContextKey | undefined; diff --git a/scripts/vscode.patch b/scripts/vscode.patch index 7b14fc7c1be1..2d66c702ecad 100644 --- a/scripts/vscode.patch +++ b/scripts/vscode.patch @@ -921,6 +921,44 @@ index 484cef9..f728fc8 100644 @@ -137 +137 @@ function connectToRenderer(protocol: IMessagePassingProtocol): Promise this.startWatching()); +@@ -56,0 +59 @@ export class FileWatcher { ++ return retry.run('Watcher'); +@@ -113 +116 @@ export class FileWatcher { +- })); ++ })).then(() => retry.recover('Watcher')); +diff --git a/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts b/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts +index 7e3a324..0bc5aac 100644 +--- a/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts ++++ b/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts +@@ -18,0 +19 @@ import { getPathFromAmdModule } from 'vs/base/common/amd'; ++const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; +@@ -36,0 +38 @@ export class FileWatcher { ++ retry.register('Watcher', () => this.startWatching()); +@@ -59,0 +62 @@ export class FileWatcher { ++ return retry.run('Watcher'); +@@ -116 +119 @@ export class FileWatcher { +- })); ++ })).then(() => retry.recover('Watcher')); +diff --git a/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts b/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts +index 74dad64..34cd83b 100644 +--- a/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts ++++ b/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts +@@ -14,0 +15 @@ import { getPathFromAmdModule } from 'vs/base/common/amd'; ++const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; +@@ -40,0 +42 @@ export class OutOfProcessWin32FolderWatcher { ++ retry.register('Watcher', () => this.startWatcher()); +@@ -52,0 +55 @@ export class OutOfProcessWin32FolderWatcher { ++ this.handle.stdout.once('data', () => retry.recover('Watcher')); +@@ -110,0 +114 @@ export class OutOfProcessWin32FolderWatcher { ++ return retry.run('Watcher'); diff --git a/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts b/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts index 3c78990..545d91a 100644 --- a/src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts From a8533a94835a8bd8c1eb5b1e9175bb6c75144cf2 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 17:32:28 -0500 Subject: [PATCH 04/14] Specify method when proxy doesn't exist --- packages/protocol/src/node/server.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/protocol/src/node/server.ts b/packages/protocol/src/node/server.ts index 238e88765155..925b848bdd1e 100644 --- a/packages/protocol/src/node/server.ts +++ b/packages/protocol/src/node/server.ts @@ -138,7 +138,7 @@ export class Server { let response: any; try { - const proxy = this.getProxy(proxyId); + const proxy = this.getProxy(proxyId, method); if (typeof proxy.instance[method] !== "function") { throw new Error(`"${method}" is not a function`); } @@ -231,7 +231,7 @@ export class Server { // It might have finished because we disposed it due to a disconnect. if (!this.disconnected) { this.sendEvent(proxyId, "done"); - this.getProxy(proxyId).disposeTimeout = setTimeout(() => { + this.getProxy(proxyId, "disposeTimeout").disposeTimeout = setTimeout(() => { instance.dispose(); this.removeProxy(proxyId); }, this.responseTimeout); @@ -317,7 +317,7 @@ export class Server { * Call after disposing a proxy. */ private removeProxy(proxyId: number | Module): void { - clearTimeout(this.getProxy(proxyId).disposeTimeout as any); + clearTimeout(this.getProxy(proxyId, "disposeTimeout").disposeTimeout as any); this.proxies.delete(proxyId); logger.trace(() => [ @@ -331,9 +331,9 @@ export class Server { return stringify(value, undefined, (p) => this.storeProxy(p)); } - private getProxy(proxyId: number | Module): ProxyData { + private getProxy(proxyId: number | Module, method: string): ProxyData { if (!this.proxies.has(proxyId)) { - throw new Error(`proxy ${proxyId} disposed too early`); + throw new Error(`Cannot run "${method}" on proxy ${proxyId}: proxy does not exist`); } return this.proxies.get(proxyId)!; From 5d7fd3057f13d3e18e8f557169c1ad0c50b986d3 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 17:32:41 -0500 Subject: [PATCH 05/14] Don't error when closing/killing disconnected proxy --- packages/protocol/src/browser/client.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 58034eb08ba0..dc19301ef7b1 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -176,6 +176,13 @@ export class Client { */ private async remoteCall(proxyId: number | Module, method: string, args: any[]): Promise { if (this.disconnected && typeof proxyId === "number") { + // Can assume killing or closing works because a disconnected proxy + // is disposed on the server's side. + switch (method) { + case "close": + case "kill": + return Promise.resolve(); + } return Promise.reject( new Error(`Unable to call "${method}" on proxy ${proxyId}: disconnected`), ); From 4ce4dbb3bc472313a5a0a42b9af71e05c4ccac1e Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 18:21:14 -0500 Subject: [PATCH 06/14] Specify proxy ID when a method doesn't exist --- packages/protocol/src/node/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protocol/src/node/server.ts b/packages/protocol/src/node/server.ts index 925b848bdd1e..1693aef85277 100644 --- a/packages/protocol/src/node/server.ts +++ b/packages/protocol/src/node/server.ts @@ -140,7 +140,7 @@ export class Server { try { const proxy = this.getProxy(proxyId, method); if (typeof proxy.instance[method] !== "function") { - throw new Error(`"${method}" is not a function`); + throw new Error(`"${method}" is not a function on proxy ${proxyId}`); } response = proxy.instance[method](...args); From d13dd87cfe014dc9e1a8fb11b8b5ffe2fb83e933 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 18:22:36 -0500 Subject: [PATCH 07/14] Use our retry for the searcher Also dispose some things for the watcher because it doesn't seem that was done properly. The searcher also now starts immediately so there won't be lag when you perform your first search. --- scripts/vscode.patch | 48 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/scripts/vscode.patch b/scripts/vscode.patch index 2d66c702ecad..e08286d88439 100644 --- a/scripts/vscode.patch +++ b/scripts/vscode.patch @@ -922,29 +922,31 @@ index 484cef9..f728fc8 100644 - process.kill(initData.parentPid, 0); // throws an exception if the main process doesn't exist anymore. + // process.kill(initData.parentPid, 0); // throws an exception if the main process doesn't exist anymore. diff --git a/src/vs/workbench/services/files/node/watcher/nsfw/watcherService.ts b/src/vs/workbench/services/files/node/watcher/nsfw/watcherService.ts -index ca03fc9..b8befc8 100644 +index ca03fc9..e3dcd08 100644 --- a/src/vs/workbench/services/files/node/watcher/nsfw/watcherService.ts +++ b/src/vs/workbench/services/files/node/watcher/nsfw/watcherService.ts @@ -18,0 +19 @@ import { getPathFromAmdModule } from 'vs/base/common/amd'; +const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; @@ -35,0 +37 @@ export class FileWatcher { + retry.register('Watcher', () => this.startWatching()); -@@ -56,0 +59 @@ export class FileWatcher { +@@ -56,0 +59,2 @@ export class FileWatcher { ++ this.toDispose = dispose(this.toDispose); + return retry.run('Watcher'); -@@ -113 +116 @@ export class FileWatcher { +@@ -113 +117 @@ export class FileWatcher { - })); + })).then(() => retry.recover('Watcher')); diff --git a/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts b/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts -index 7e3a324..0bc5aac 100644 +index 7e3a324..b9ccd63 100644 --- a/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts +++ b/src/vs/workbench/services/files/node/watcher/unix/watcherService.ts @@ -18,0 +19 @@ import { getPathFromAmdModule } from 'vs/base/common/amd'; +const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; @@ -36,0 +38 @@ export class FileWatcher { + retry.register('Watcher', () => this.startWatching()); -@@ -59,0 +62 @@ export class FileWatcher { +@@ -59,0 +62,2 @@ export class FileWatcher { ++ this.toDispose = dispose(this.toDispose); + return retry.run('Watcher'); -@@ -116 +119 @@ export class FileWatcher { +@@ -116 +120 @@ export class FileWatcher { - })); + })).then(() => retry.recover('Watcher')); diff --git a/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts b/src/vs/workbench/services/files/node/watcher/win32/csharpWatcherService.ts @@ -969,6 +971,40 @@ index 3c78990..545d91a 100644 @@ -130 +130 @@ export class KeyboardMapperFactory { - if (OS === OperatingSystem.Windows) { + if (isNative && OS === OperatingSystem.Windows) { +diff --git a/src/vs/workbench/services/search/node/searchService.ts b/src/vs/workbench/services/search/node/searchService.ts +index 3eaafa4..0345bad 100644 +--- a/src/vs/workbench/services/search/node/searchService.ts ++++ b/src/vs/workbench/services/search/node/searchService.ts +@@ -11 +11 @@ import { Event } from 'vs/base/common/event'; +-import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; ++import { Disposable, IDisposable, toDisposable, dispose } from 'vs/base/common/lifecycle'; +@@ -32,0 +33 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur ++const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; +@@ -433,0 +435 @@ export class DiskSearch implements ISearchResultProvider { ++ private toDispose: IDisposable[] = []; +@@ -470,6 +472,16 @@ export class DiskSearch implements ISearchResultProvider { +- const client = new Client( +- getPathFromAmdModule(require, 'bootstrap-fork'), +- opts); +- +- const channel = getNextTickChannel(client.getChannel('search')); +- this.raw = new SearchChannelClient(channel); ++ const connect = (): void => { ++ const client = new Client( ++ getPathFromAmdModule(require, 'bootstrap-fork'), ++ opts); ++ client.onDidProcessExit(() => { ++ this.toDispose = dispose(this.toDispose); ++ retry.run('Searcher'); ++ }, null, this.toDispose); ++ this.toDispose.push(client); ++ ++ const channel = getNextTickChannel(client.getChannel('search')); ++ this.raw = new SearchChannelClient(channel); ++ this.raw.clearCache('test-connectivity').then(() => retry.recover('Searcher')); ++ }; ++ retry.register('Searcher', connect); ++ connect(); diff --git a/src/vs/workbench/services/timer/electron-browser/timerService.ts b/src/vs/workbench/services/timer/electron-browser/timerService.ts index 6e6fbcc..645bd72 100644 --- a/src/vs/workbench/services/timer/electron-browser/timerService.ts From 02f215d97fd0354777b9d09a85a4559c2658bb6f Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 18:55:26 -0500 Subject: [PATCH 08/14] Use our retry for the extension host --- scripts/vscode.patch | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/vscode.patch b/scripts/vscode.patch index e08286d88439..cb3eb1e9f562 100644 --- a/scripts/vscode.patch +++ b/scripts/vscode.patch @@ -914,6 +914,18 @@ index 0592910..0ce7e35 100644 +++ b/src/vs/workbench/services/extensions/electron-browser/cachedExtensionScanner.ts @@ -33,0 +34 @@ function getSystemExtensionsRoot(): string { + return (require('vs/../../../../packages/vscode/src/fill/paths') as typeof import ('vs/../../../../packages/vscode/src/fill/paths')).getBuiltInExtensionsDirectory(); +diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts +index 2c2f9c7..69fa321 100644 +--- a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts ++++ b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts +@@ -34,0 +35 @@ import { Schemas } from 'vs/base/common/network'; ++const retry = (require('vs/../../../../packages/vscode/src/workbench') as typeof import ('vs/../../../../packages/vscode/src/workbench')).workbench.retry; +@@ -117,0 +119 @@ export class ExtensionService extends Disposable implements IExtensionService { ++ retry.register('Extension Host', () => this.startExtensionHost()); +@@ -435,0 +438 @@ export class ExtensionService extends Disposable implements IExtensionService { ++ extHostProcessWorker.start().then(() => retry.recover('Extension Host')); +@@ -458,0 +462 @@ export class ExtensionService extends Disposable implements IExtensionService { ++ return retry.run('Extension Host'); diff --git a/src/vs/workbench/services/extensions/node/extensionHostProcess.ts b/src/vs/workbench/services/extensions/node/extensionHostProcess.ts index 484cef9..f728fc8 100644 --- a/src/vs/workbench/services/extensions/node/extensionHostProcess.ts From 091da77eee4a2896ab5407d36f8c205e6f0c1fbb Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 10:12:45 -0500 Subject: [PATCH 09/14] Emit error in parent proxy class Reduces duplicate code. Not all items are "supposed" to have an error event according to the original implementation we are filling, but there is no reason why we can't emit our own events (and are already doing so for the "disconnected" event anyway). --- .../src/browser/modules/child_process.ts | 7 +------ packages/protocol/src/browser/modules/fs.ts | 7 +------ packages/protocol/src/browser/modules/net.ts | 7 +------ .../protocol/src/browser/modules/stream.ts | 18 ++++-------------- packages/protocol/src/common/proxy.ts | 11 +++++++++-- 5 files changed, 16 insertions(+), 34 deletions(-) diff --git a/packages/protocol/src/browser/modules/child_process.ts b/packages/protocol/src/browser/modules/child_process.ts index 17e90b48024e..d0f620076f3a 100644 --- a/packages/protocol/src/browser/modules/child_process.ts +++ b/packages/protocol/src/browser/modules/child_process.ts @@ -88,12 +88,7 @@ export class ChildProcess extends ClientProxy implements cp.C return true; // Always true since we can't get this synchronously. } - protected handleDisconnect(error: Error): void { - try { - this.emit("error", error); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } + protected handleDisconnect(): void { this.emit("exit", 1); this.emit("close"); } diff --git a/packages/protocol/src/browser/modules/fs.ts b/packages/protocol/src/browser/modules/fs.ts index a25ee2d0af60..c1a2b8e73ff1 100644 --- a/packages/protocol/src/browser/modules/fs.ts +++ b/packages/protocol/src/browser/modules/fs.ts @@ -42,12 +42,7 @@ class Watcher extends ClientProxy implements fs.FSWatcher { this.proxy.close(); } - protected handleDisconnect(error: Error): void { - try { - this.emit("error", error); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } + protected handleDisconnect(): void { this.emit("close"); } } diff --git a/packages/protocol/src/browser/modules/net.ts b/packages/protocol/src/browser/modules/net.ts index bda36c94adc2..0d8354c4ff26 100644 --- a/packages/protocol/src/browser/modules/net.ts +++ b/packages/protocol/src/browser/modules/net.ts @@ -207,12 +207,7 @@ export class Server extends ClientProxy implements net.Server { cb(null, this.sockets.size); } - protected handleDisconnect(error: Error): void { - try { - this.emit("error", error); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } + protected handleDisconnect(): void { this.emit("close"); } } diff --git a/packages/protocol/src/browser/modules/stream.ts b/packages/protocol/src/browser/modules/stream.ts index eee58e013147..c636cf326e38 100644 --- a/packages/protocol/src/browser/modules/stream.ts +++ b/packages/protocol/src/browser/modules/stream.ts @@ -82,12 +82,7 @@ export class Writable extends ClientPro }); } - protected handleDisconnect(error: Error): void { - try { - this.emit("error", error); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } + protected handleDisconnect(): void { this.emit("close"); this.emit("finish"); } @@ -165,12 +160,7 @@ export class Readable extends ClientP return this; } - protected handleDisconnect(error: Error): void { - try { - this.emit("error", error); - } catch (error) { - // If nothing is listening, EventEmitter will throw an error. - } + protected handleDisconnect(): void { this.emit("close"); this.emit("end"); } @@ -251,8 +241,8 @@ export class Duplex extends Writable imp return this; } - protected handleDisconnect(error: Error): void { - super.handleDisconnect(error); + protected handleDisconnect(): void { + super.handleDisconnect(); this.emit("end"); } } diff --git a/packages/protocol/src/common/proxy.ts b/packages/protocol/src/common/proxy.ts index c91e295f51bd..4a9604e4e1c3 100644 --- a/packages/protocol/src/common/proxy.ts +++ b/packages/protocol/src/common/proxy.ts @@ -42,7 +42,14 @@ export abstract class ClientProxy extends EventEmitter { super(); this.initialize(proxyPromise); if (this.bindEvents) { - this.on("disconnected", (error) => this.handleDisconnect(error)); + this.on("disconnected", (error) => { + try { + this.emit("error", error); + } catch (error) { + // If nothing is listening, EventEmitter will throw an error. + } + this.handleDisconnect(); + }); } } @@ -63,7 +70,7 @@ export abstract class ClientProxy extends EventEmitter { } } - protected abstract handleDisconnect(error: Error): void; + protected abstract handleDisconnect(): void; } /** From 99afcf7f7f85ed88436725fb39eb3097cf9fb4e8 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 10:30:25 -0500 Subject: [PATCH 10/14] Reconnect spdlog --- packages/protocol/src/browser/modules/spdlog.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/protocol/src/browser/modules/spdlog.ts b/packages/protocol/src/browser/modules/spdlog.ts index 225a58089c2e..80fc5dde2920 100644 --- a/packages/protocol/src/browser/modules/spdlog.ts +++ b/packages/protocol/src/browser/modules/spdlog.ts @@ -3,6 +3,16 @@ import { ClientProxy } from "../../common/proxy"; import { RotatingLoggerProxy, SpdlogModuleProxy } from "../../node/modules/spdlog"; class RotatingLogger extends ClientProxy implements spdlog.RotatingLogger { + public constructor( + private readonly moduleProxy: SpdlogModuleProxy, + private readonly name: string, + private readonly filename: string, + private readonly filesize: number, + private readonly filecount: number, + ) { + super(moduleProxy.createLogger(name, filename, filesize, filecount)); + } + public async trace (message: string): Promise { this.proxy.trace(message); } public async debug (message: string): Promise { this.proxy.debug(message); } public async info (message: string): Promise { this.proxy.info(message); } @@ -15,7 +25,7 @@ class RotatingLogger extends ClientProxy implements spdlog. public async drop (): Promise { this.proxy.drop(); } protected handleDisconnect(): void { - // TODO: reconnect. + this.initialize(this.moduleProxy.createLogger(this.name, this.filename, this.filesize, this.filecount)); } } @@ -25,7 +35,7 @@ export class SpdlogModule { public constructor(private readonly proxy: SpdlogModuleProxy) { this.RotatingLogger = class extends RotatingLogger { public constructor(name: string, filename: string, filesize: number, filecount: number) { - super(proxy.createLogger(name, filename, filesize, filecount)); + super(proxy, name, filename, filesize, filecount); } }; } From 64936003172c96eb02f9baf1cbed9153379585bd Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 11:34:25 -0500 Subject: [PATCH 11/14] Add error message when shared process disconnects --- scripts/vscode.patch | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/vscode.patch b/scripts/vscode.patch index cb3eb1e9f562..b24e38f87495 100644 --- a/scripts/vscode.patch +++ b/scripts/vscode.patch @@ -883,7 +883,7 @@ index acb68c8..bee143a 100644 - !isMacintosh || // macOS only + !browser.isMacintosh || // macOS only diff --git a/src/vs/workbench/electron-browser/workbench.ts b/src/vs/workbench/electron-browser/workbench.ts -index 7445d7b..0291dee 100644 +index 7445d7b..ba6bf4b 100644 --- a/src/vs/workbench/electron-browser/workbench.ts +++ b/src/vs/workbench/electron-browser/workbench.ts @@ -19 +19,2 @@ import { Registry } from 'vs/platform/registry/common/platform'; @@ -899,13 +899,15 @@ index 7445d7b..0291dee 100644 @@ -458 +462 @@ export class Workbench extends Disposable implements IPartService { - addClasses(document.body, platformClass); // used by our fonts + addClasses(document.body, platformClass, isWeb ? 'web' : 'native'); // used by our fonts -@@ -633 +637 @@ export class Workbench extends Disposable implements IPartService { +@@ -491,0 +496 @@ export class Workbench extends Disposable implements IPartService { ++ client.onClose(() => this.notificationService.error("Disconnected from shared process. Searching, installing, enabling, and disabling extensions will not work until the page is refreshed.")); +@@ -633 +638 @@ export class Workbench extends Disposable implements IPartService { - if (!isMacintosh && this.useCustomTitleBarStyle()) { + if (isWeb || (!isMacintosh && this.useCustomTitleBarStyle())) { -@@ -1241 +1245 @@ export class Workbench extends Disposable implements IPartService { +@@ -1241 +1246 @@ export class Workbench extends Disposable implements IPartService { - if ((isWindows || isLinux) && this.useCustomTitleBarStyle()) { + if ((isWeb || isWindows || isLinux) && this.useCustomTitleBarStyle()) { -@@ -1397 +1401 @@ export class Workbench extends Disposable implements IPartService { +@@ -1397 +1402 @@ export class Workbench extends Disposable implements IPartService { - } else if (isMacintosh) { + } else if (isNative && isMacintosh) { diff --git a/src/vs/workbench/services/extensions/electron-browser/cachedExtensionScanner.ts b/src/vs/workbench/services/extensions/electron-browser/cachedExtensionScanner.ts From 6b9f29f4459913ab35e972ff282cc1bfbcacb0e1 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 16:18:17 -0500 Subject: [PATCH 12/14] Pass method resolve to parse --- packages/protocol/src/browser/client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index dc19301ef7b1..0139effbac86 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -174,7 +174,7 @@ export class Client { /** * Make a remote call for a proxy's method using proto. */ - private async remoteCall(proxyId: number | Module, method: string, args: any[]): Promise { + private remoteCall(proxyId: number | Module, method: string, args: any[]): Promise { if (this.disconnected && typeof proxyId === "number") { // Can assume killing or closing works because a disconnected proxy // is disposed on the server's side. @@ -247,7 +247,7 @@ export class Client { const d1 = this.successEmitter.event(id, (message) => { dispose(); - resolve(this.parse(message.getResponse())); + resolve(this.parse(message.getResponse(), promise)); }); const d2 = this.failEmitter.event(id, (message) => { From 4522be1227a94a280dc374c46b7da2f7d0f663ee Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 16:33:02 -0500 Subject: [PATCH 13/14] Don't pass method to getProxy It doesn't tell you anything that trace logging wouldn't and has no relation to what the function actually does. --- packages/protocol/src/node/server.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/protocol/src/node/server.ts b/packages/protocol/src/node/server.ts index 1693aef85277..489d57115f7e 100644 --- a/packages/protocol/src/node/server.ts +++ b/packages/protocol/src/node/server.ts @@ -138,7 +138,7 @@ export class Server { let response: any; try { - const proxy = this.getProxy(proxyId, method); + const proxy = this.getProxy(proxyId); if (typeof proxy.instance[method] !== "function") { throw new Error(`"${method}" is not a function on proxy ${proxyId}`); } @@ -231,7 +231,7 @@ export class Server { // It might have finished because we disposed it due to a disconnect. if (!this.disconnected) { this.sendEvent(proxyId, "done"); - this.getProxy(proxyId, "disposeTimeout").disposeTimeout = setTimeout(() => { + this.getProxy(proxyId).disposeTimeout = setTimeout(() => { instance.dispose(); this.removeProxy(proxyId); }, this.responseTimeout); @@ -317,7 +317,7 @@ export class Server { * Call after disposing a proxy. */ private removeProxy(proxyId: number | Module): void { - clearTimeout(this.getProxy(proxyId, "disposeTimeout").disposeTimeout as any); + clearTimeout(this.getProxy(proxyId).disposeTimeout as any); this.proxies.delete(proxyId); logger.trace(() => [ @@ -331,9 +331,9 @@ export class Server { return stringify(value, undefined, (p) => this.storeProxy(p)); } - private getProxy(proxyId: number | Module, method: string): ProxyData { + private getProxy(proxyId: number | Module): ProxyData { if (!this.proxies.has(proxyId)) { - throw new Error(`Cannot run "${method}" on proxy ${proxyId}: proxy does not exist`); + throw new Error(`proxy ${proxyId} disposed too early`); } return this.proxies.get(proxyId)!; From 08cbc228246fa7b01e06a3c132ea7b43898103c1 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 28 Mar 2019 17:32:27 -0500 Subject: [PATCH 14/14] Fix infinite recursion when disposing protocol client in tests --- packages/protocol/src/browser/client.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/protocol/src/browser/client.ts b/packages/protocol/src/browser/client.ts index 0139effbac86..30239739b3a4 100644 --- a/packages/protocol/src/browser/client.ts +++ b/packages/protocol/src/browser/client.ts @@ -143,7 +143,12 @@ export class Client { clearTimeout(this.pingTimeout as any); this.pingTimeout = undefined; handleDisconnect(); - this.dispose(); + this.proxies.clear(); + this.successEmitter.dispose(); + this.failEmitter.dispose(); + this.eventEmitter.dispose(); + this.initDataEmitter.dispose(); + this.sharedProcessActiveEmitter.dispose(); }); connection.onUp(() => this.disconnected = false); @@ -159,12 +164,6 @@ export class Client { */ public dispose(): void { this.connection.close(); - this.proxies.clear(); - this.successEmitter.dispose(); - this.failEmitter.dispose(); - this.eventEmitter.dispose(); - this.initDataEmitter.dispose(); - this.sharedProcessActiveEmitter.dispose(); } public get initData(): Promise {