-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Running heavy filesystem operations in separate processes #899
Changes from all commits
8ba39dc
102e9e4
ad82299
95f2e9f
50274d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,4 +15,5 @@ examples/*/webpack.config.js | |
.browser_modules | ||
**/docs/api | ||
package-backup.json | ||
.history | ||
.history | ||
.Trash-* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* | ||
* Copyright (C) 2017 TypeFox and others. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
*/ | ||
|
||
import { Message } from "vscode-jsonrpc"; | ||
import { ILogger } from "../../common"; | ||
|
||
export interface ResolvedConnectionErrorHandlerOptions { | ||
readonly serverName: string | ||
readonly logger: ILogger | ||
/** | ||
* The maximum amout of errors allowed before stopping the server. | ||
*/ | ||
readonly maxErrors: number | ||
/** | ||
* The maimum amount of restarts allowed in the restart interval. | ||
*/ | ||
readonly maxRestarts: number | ||
/** | ||
* In minutes. | ||
*/ | ||
readonly restartInterval: number | ||
} | ||
|
||
export type ConnectionErrorHandlerOptions = Partial<ResolvedConnectionErrorHandlerOptions> & { | ||
readonly serverName: string | ||
readonly logger: ILogger | ||
}; | ||
|
||
export class ConnectionErrorHandler { | ||
|
||
protected readonly options: ResolvedConnectionErrorHandlerOptions; | ||
constructor(options: ConnectionErrorHandlerOptions) { | ||
this.options = { | ||
maxErrors: 3, | ||
maxRestarts: 5, | ||
restartInterval: 3, | ||
...options | ||
}; | ||
} | ||
|
||
shouldStop(error: Error, message?: Message, count?: number): boolean { | ||
return !count || count > this.options.maxErrors; | ||
} | ||
|
||
protected readonly restarts: number[] = []; | ||
shouldRestart(): boolean { | ||
this.restarts.push(Date.now()); | ||
if (this.restarts.length <= this.options.maxRestarts) { | ||
return true; | ||
} | ||
const diff = this.restarts[this.restarts.length - 1] - this.restarts[0]; | ||
if (diff <= this.options.restartInterval * 60 * 1000) { | ||
// tslint:disable-next-line:max-line-length | ||
this.options.logger.error(`The ${this.options.serverName} server crashed ${this.options.maxRestarts} times in the last ${this.options.restartInterval} minutes. The server will not be restarted.`); | ||
return false; | ||
} | ||
this.restarts.shift(); | ||
return true; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,4 @@ | |
|
||
export * from './handler'; | ||
export * from './proxy-factory'; | ||
export * from './connection-error-handler'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,3 +6,4 @@ | |
*/ | ||
|
||
export * from './connection'; | ||
export * from './ipc-connection-provider'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/* | ||
* Copyright (C) 2017 TypeFox and others. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
*/ | ||
|
||
import 'reflect-metadata'; | ||
import { ConsoleLogger } from 'vscode-ws-jsonrpc/lib/logger'; | ||
import { createMessageConnection, IPCMessageReader, IPCMessageWriter, Trace } from 'vscode-jsonrpc'; | ||
import { THEIA_PARENT_PID, THEIA_ENTRY_POINT, IPCEntryPoint } from './ipc-protocol'; | ||
|
||
/** | ||
* Exit the current process if the parent process is not alive. | ||
* Relevant only for some OS, like Windows | ||
*/ | ||
if (process.env[THEIA_PARENT_PID]) { | ||
const parentPid = Number(process.env[THEIA_PARENT_PID]); | ||
|
||
if (typeof parentPid === 'number' && !isNaN(parentPid)) { | ||
setInterval(function () { | ||
try { | ||
// throws an exception if the main process doesn't exist anymore. | ||
process.kill(parentPid, 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From: https://nodejs.org/dist/latest-v8.x/docs/api/child_process.html#child_process_subprocess_kill_signal Moreover 0 is not a valid signal number on linux at least, not sure what is supposed to happen. The parent should just notify the child as it exists no ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think similar work was done for the LSPs recently with onStop() ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this code is copied from vscode, they use it to ensure that child processes exit if the main process crashed and did not kill children, make sense only on windows, other OSs take care about child processes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK thx could you add a comment in the code about it please? it being windows specific? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note I don't think child processes exit when the parent exists on linux. You have to do it yourself like in the LSP case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will add, so far we experience it only on windows |
||
} catch (e) { | ||
process.exit(); | ||
} | ||
}, 5000); | ||
} | ||
} | ||
|
||
const reader = new IPCMessageReader(process); | ||
const writer = new IPCMessageWriter(process); | ||
const logger = new ConsoleLogger(); | ||
const connection = createMessageConnection(reader, writer, logger); | ||
connection.trace(Trace.Off, { | ||
log: (message, data) => console.log(`${message} ${data}`) | ||
}); | ||
|
||
const entryPoint = require(process.env[THEIA_ENTRY_POINT]!).default as IPCEntryPoint; | ||
entryPoint(connection); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
/* | ||
* Copyright (C) 2017 TypeFox and others. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
*/ | ||
|
||
import * as path from "path"; | ||
import * as cp from "child_process"; | ||
import { injectable, inject } from "inversify"; | ||
import { Trace, IPCMessageReader, IPCMessageWriter, createMessageConnection, MessageConnection, Message } from "vscode-jsonrpc"; | ||
import { ILogger, ConnectionErrorHandler, DisposableCollection, Disposable } from "../../common"; | ||
import { THEIA_PARENT_PID, THEIA_ENTRY_POINT } from './ipc-protocol'; | ||
|
||
export interface ResolvedIPCConnectionOptions { | ||
readonly serverName: string | ||
readonly entryPoint: string | ||
readonly logger: ILogger | ||
readonly args: string[] | ||
readonly debug?: number | ||
readonly debugBrk?: number | ||
readonly errorHandler?: ConnectionErrorHandler | ||
} | ||
export type IPCConnectionOptions = Partial<ResolvedIPCConnectionOptions> & { | ||
readonly serverName: string | ||
readonly entryPoint: string | ||
}; | ||
|
||
@injectable() | ||
export class IPCConnectionProvider { | ||
|
||
@inject(ILogger) | ||
protected readonly logger: ILogger; | ||
|
||
listen(options: IPCConnectionOptions, acceptor: (connection: MessageConnection) => void): Disposable { | ||
return this.doListen({ | ||
logger: this.logger, | ||
args: [], | ||
...options | ||
}, acceptor); | ||
} | ||
|
||
protected doListen(options: ResolvedIPCConnectionOptions, acceptor: (connection: MessageConnection) => void): Disposable { | ||
const childProcess = this.fork(options); | ||
const connection = this.createConnection(childProcess, options); | ||
const toStop = new DisposableCollection(); | ||
const toCancelStop = toStop.push(Disposable.create(() => childProcess.kill())); | ||
const errorHandler = options.errorHandler; | ||
if (errorHandler) { | ||
connection.onError((e: [Error, Message | undefined, number | undefined]) => { | ||
if (errorHandler.shouldStop(e[0], e[1], e[2])) { | ||
toStop.dispose(); | ||
} | ||
}); | ||
connection.onClose(() => { | ||
if (toStop.disposed) { | ||
return; | ||
} | ||
if (errorHandler.shouldRestart()) { | ||
toCancelStop.dispose(); | ||
toStop.push(this.doListen(options, acceptor)); | ||
} | ||
}); | ||
} | ||
acceptor(connection); | ||
return toStop; | ||
} | ||
|
||
protected createConnection(childProcess: cp.ChildProcess, options: ResolvedIPCConnectionOptions): MessageConnection { | ||
const reader = new IPCMessageReader(childProcess); | ||
const writer = new IPCMessageWriter(childProcess); | ||
const connection = createMessageConnection(reader, writer, { | ||
error: (message: string) => this.logger.error(`[${options.serverName}: ${childProcess.pid}] ${message}`), | ||
warn: (message: string) => this.logger.warn(`[${options.serverName}: ${childProcess.pid}] ${message}`), | ||
info: (message: string) => this.logger.info(`[${options.serverName}: ${childProcess.pid}] ${message}`), | ||
log: (message: string) => this.logger.info(`[${options.serverName}: ${childProcess.pid}] ${message}`) | ||
}); | ||
connection.trace(Trace.Off, { | ||
log: (message, data) => this.logger.info(`[${options.serverName}: ${childProcess.pid}] ${message} ${data}`) | ||
}); | ||
return connection; | ||
} | ||
|
||
protected fork(options: ResolvedIPCConnectionOptions): cp.ChildProcess { | ||
const forkOptions: cp.ForkOptions = { | ||
silent: true, | ||
env: { | ||
...process.env | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just asking to squeeze some additional knowledge from you: it is the same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, it copies properties to a new object, later 2 more properties added to the new object but not to |
||
execArgv: [] | ||
}; | ||
forkOptions.env[THEIA_PARENT_PID] = String(process.pid); | ||
forkOptions.env[THEIA_ENTRY_POINT] = options.entryPoint; | ||
if (typeof options.debug === 'number') { | ||
forkOptions.execArgv = ['--nolazy', '--inspect=' + options.debug]; | ||
} | ||
if (typeof options.debugBrk === 'number') { | ||
forkOptions.execArgv = ['--nolazy', '--inspect-brk=' + options.debugBrk]; | ||
} | ||
const childProcess = cp.fork(path.resolve(__dirname, 'ipc-bootstrap.js'), options.args, forkOptions); | ||
childProcess.stdout.on('data', data => this.logger.info(`[${options.serverName}: ${childProcess.pid}] ${data.toString()}`)); | ||
childProcess.stderr.on('data', data => this.logger.error(`[${options.serverName}: ${childProcess.pid}] ${data.toString()}`)); | ||
|
||
this.logger.debug(`[${options.serverName}: ${childProcess.pid}] IPC started`); | ||
childProcess.once('exit', () => this.logger.debug(`[${options.serverName}: ${childProcess.pid}] IPC exited`)); | ||
|
||
return childProcess; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
|
||
/* | ||
* Copyright (C) 2017 TypeFox and others. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
*/ | ||
|
||
import { MessageConnection } from "vscode-jsonrpc"; | ||
|
||
export const THEIA_PARENT_PID = 'THEIA_PARENT_PID'; | ||
export const THEIA_ENTRY_POINT = 'THEIA_ENTRY_POINT'; | ||
|
||
export type IPCEntryPoint = (connection: MessageConnection) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will leak restarts we should always discard all restarts < last restart - interval
We should also do this all the time before return.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There never will be restarts more than
maxRestarts
. AftermaxRestarts
reached for eachpush
there is oneshift
or a server should be stopped completely.