diff --git a/src/features/commands.ts b/src/features/commands.ts index 95dc512b0..166e1c466 100644 --- a/src/features/commands.ts +++ b/src/features/commands.ts @@ -97,14 +97,14 @@ async function pickProjectAndStart(server: OmniSharpServer, optionProvider: Opti } export async function showProjectSelector(server: OmniSharpServer, targets: LaunchTarget[]): Promise { - return vscode.window.showQuickPick(targets, { + const launchTarget = await vscode.window.showQuickPick(targets, { matchOnDescription: true, placeHolder: `Select 1 of ${targets.length} projects` - }).then(async launchTarget => { - if (launchTarget) { - return server.restart(launchTarget); - } }); + + if (launchTarget !== undefined) { + return server.restart(launchTarget); + } } interface Command { diff --git a/src/observers/OmnisharpDebugModeLoggerObserver.ts b/src/observers/OmnisharpDebugModeLoggerObserver.ts index 67dbc350e..6bea8fca2 100644 --- a/src/observers/OmnisharpDebugModeLoggerObserver.ts +++ b/src/observers/OmnisharpDebugModeLoggerObserver.ts @@ -57,7 +57,7 @@ export class OmnisharpDebugModeLoggerObserver extends BaseLoggerObserver { } private handleOmnisharpServerRequestCancelled(event: OmnisharpServerRequestCancelled) { - this.logger.appendLine(`Cancelled request for ${event.command} (${event.id}).`); + this.logger.appendLine(`Cancelled request for ${event.command}${event.id ? ` (${event.id})` : ''}.`); this.logger.appendLine(); } @@ -101,4 +101,4 @@ export class OmnisharpDebugModeLoggerObserver extends BaseLoggerObserver { default: throw new Error(`Unknown log level value: ${logLevel}`); } } -} \ No newline at end of file +} diff --git a/src/omnisharp/loggingEvents.ts b/src/omnisharp/loggingEvents.ts index dc463ec63..89d843831 100644 --- a/src/omnisharp/loggingEvents.ts +++ b/src/omnisharp/loggingEvents.ts @@ -125,7 +125,7 @@ export class OmnisharpServerDequeueRequest implements BaseEvent { export class OmnisharpServerRequestCancelled implements BaseEvent { type = EventType.OmnisharpServerRequestCancelled; - constructor(public command: string, public id: number) { } + constructor(public command: string, public id: number | undefined) { } } export class OmnisharpServerProcessRequestStart implements BaseEvent { diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index fd82bba9b..db32db004 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -12,7 +12,7 @@ import * as serverUtils from '../omnisharp/utils'; import { vscode, CancellationToken } from '../vscodeAdapter'; import { ChildProcess, exec } from 'child_process'; import { LaunchTarget, findLaunchTargets, LaunchTargetKind } from './launcher'; -import { ReadLine, createInterface } from 'readline'; +import { createInterface } from 'readline'; import { Request, RequestQueueCollection } from './requestQueue'; import { DelayTracker } from './delayTracker'; import { EventEmitter } from 'events'; @@ -40,6 +40,18 @@ enum ServerState { Stopped } +type State = { + status: ServerState.Stopped, +} | { + status: ServerState.Starting, + disposables: CompositeDisposable, +} | { + status: ServerState.Started, + disposables: CompositeDisposable, + serverProcess: ChildProcess, + telemetryIntervalId: NodeJS.Timeout, +}; + module Events { export const StateChanged = 'stateChanged'; @@ -83,17 +95,13 @@ const latestVersionFileServerPath = 'releases/versioninfo.txt'; export class OmniSharpServer { private static _nextId = 1; - private _readLine: ReadLine; - private _disposables: CompositeDisposable; - private _delayTrackers!: { [requestName: string]: DelayTracker }; // Initialized via _start - private _telemetryIntervalId: NodeJS.Timer | undefined; + private _delayTrackers: { [requestName: string]: DelayTracker } = {}; private _eventBus = new EventEmitter(); - private _state: ServerState = ServerState.Stopped; + private _state: State = { status: ServerState.Stopped }; private _launchTarget: LaunchTarget | undefined; private _requestQueue: RequestQueueCollection; - private _serverProcess: ChildProcess; private _sessionProperties: { [key: string]: any } = {}; private _omnisharpManager: OmnisharpManager; @@ -123,7 +131,7 @@ export class OmniSharpServer { } public isRunning(): boolean { - return this._state === ServerState.Started; + return this._state.status === ServerState.Started; } public async waitForEmptyEventQueue(): Promise { @@ -133,10 +141,10 @@ export class OmniSharpServer { } } - private _setState(value: ServerState): void { - if (typeof value !== 'undefined' && value !== this._state) { - this._state = value; - this._fireEvent(Events.StateChanged, this._state); + private _setState(state: State): void { + if (state.status !== this._state.status) { + this._state = state; + this._fireEvent(Events.StateChanged, this._state.status); } } @@ -258,8 +266,8 @@ export class OmniSharpServer { // --- start, stop, and connect - private async _start(launchTarget: LaunchTarget, options: Options): Promise { - if (this._state != ServerState.Stopped) { + private async _start(launchTarget: LaunchTarget): Promise { + if (this._state.status !== ServerState.Stopped) { this.eventStream.post(new ObservableEvents.OmnisharpServerOnServerError("Attempt to start OmniSharp server failed because another server instance is running.")); return; } @@ -269,7 +277,7 @@ export class OmniSharpServer { return; } - let disposables = new CompositeDisposable(); + const disposables = new CompositeDisposable(); disposables.add(this.onServerError(err => this.eventStream.post(new ObservableEvents.OmnisharpServerOnServerError(err)) @@ -323,15 +331,18 @@ export class OmniSharpServer { disposables.add(this.onProjectChange(this.debounceUpdateProjectWithLeadingTrue)); disposables.add(this.onProjectRemoved(this.debounceUpdateProjectWithLeadingTrue)); - this._disposables = disposables; - - this._setState(ServerState.Starting); + this._setState({ + status: ServerState.Starting, + disposables, + }); this._launchTarget = launchTarget; const solutionPath = launchTarget.target; const cwd = path.dirname(solutionPath); - let args = [ + const options = this.optionProvider.GetLatestOptions(); + + const args = [ '-z', '-s', solutionPath, '--hostPID', process.pid.toString(), @@ -435,14 +446,17 @@ export class OmniSharpServer { } } - this._serverProcess = launchResult.process; this._delayTrackers = {}; - await this._doConnect(options); - this._setState(ServerState.Started); + await this._doConnect(disposables, launchResult.process, options); + this._setState({ + status: ServerState.Started, + disposables, + serverProcess: launchResult.process, + telemetryIntervalId: setInterval(() => this._reportTelemetry(), TelemetryReportingDelay), + }); this._fireEvent(Events.ServerStart, solutionPath); - this._telemetryIntervalId = setInterval(() => this._reportTelemetry(), TelemetryReportingDelay); this._requestQueue.drain(); } catch (err) { @@ -475,65 +489,57 @@ export class OmniSharpServer { } public async stop(): Promise { - - let cleanupPromise: Promise; + // We're already stopped, nothing to do :). + if (this._state.status === ServerState.Stopped) { + return; + } // Clear the session properties when the session ends. this._sessionProperties = {}; - if (this._telemetryIntervalId !== undefined) { - // Stop reporting telemetry - clearInterval(this._telemetryIntervalId); - this._telemetryIntervalId = undefined; + if (this._state.status === ServerState.Started) { + const { serverProcess, telemetryIntervalId } = this._state; + + clearInterval(telemetryIntervalId); this._reportTelemetry(); - } - if (!this._serverProcess) { - // nothing to kill - cleanupPromise = Promise.resolve(); - } - else if (process.platform === 'win32') { - // when killing a process in windows its child - // processes are *not* killed but become root - // processes. Therefore we use TASKKILL.EXE - cleanupPromise = new Promise((resolve, reject) => { - const killer = exec(`taskkill /F /T /PID ${this._serverProcess.pid}`, (err, stdout, stderr) => { - if (err) { - return reject(err); - } - }); + if (process.platform === 'win32') { + // when killing a process in windows its child + // processes are *not* killed but become root + // processes. Therefore we use TASKKILL.EXE + await new Promise((resolve, reject) => { + const killer = exec(`taskkill /F /T /PID ${serverProcess.pid}`, (err, stdout, stderr) => { + if (err) { + return reject(err); + } + }); - killer.on('exit', resolve); - killer.on('error', reject); - }); - } - else { - // Kill Unix process and children - cleanupPromise = utils.getUnixChildProcessIds(this._serverProcess.pid) - .then(children => { - for (let child of children) { - process.kill(child, 'SIGTERM'); - } - - this._serverProcess.kill('SIGTERM'); + killer.on('exit', resolve); + killer.on('error', reject); }); + } + else { + // Kill Unix process and children + const children = await utils.getUnixChildProcessIds(serverProcess.pid); + for (const child of children) { + process.kill(child, 'SIGTERM'); + } + + serverProcess.kill('SIGTERM'); + } } - let disposables = this._disposables; - this._disposables = null; + const { disposables } = this._state; - return cleanupPromise.then(() => { - this._serverProcess = null; - this._setState(ServerState.Stopped); - this._fireEvent(Events.ServerStop, this); - if (disposables) { - disposables.dispose(); - } - }); + this._setState({ status: ServerState.Stopped }); + this._fireEvent(Events.ServerStop, this); + + // Dispose of the disposables only _after_ we've fired the last server event. + disposables.dispose(); } public async restart(launchTarget: LaunchTarget | undefined = this._launchTarget): Promise { - if (this._state == ServerState.Starting) { + if (this._state.status === ServerState.Starting) { this.eventStream.post(new ObservableEvents.OmnisharpServerOnServerError("Attempt to restart OmniSharp server failed because another server instance is starting.")); return; } @@ -541,78 +547,76 @@ export class OmniSharpServer { if (launchTarget !== undefined) { await this.stop(); this.eventStream.post(new ObservableEvents.OmnisharpRestart()); - const options = this.optionProvider.GetLatestOptions(); - await this._start(launchTarget, options); + await this._start(launchTarget); } } - public autoStart(preferredPath: string): Thenable { + public async autoStart(preferredPath: string): Promise { const options = this.optionProvider.GetLatestOptions(); - return findLaunchTargets(options).then(async launchTargets => { - // If there aren't any potential launch targets, we create file watcher and try to - // start the server again once a *.sln, *.slnf, *.csproj, project.json, CSX or Cake file is created. - if (launchTargets.length === 0) { - return new Promise((resolve, reject) => { - // 1st watch for files - let watcher = this.vscode.workspace.createFileSystemWatcher('{**/*.sln,**/*.slnf,**/*.csproj,**/project.json,**/*.csx,**/*.cake}', - /*ignoreCreateEvents*/ false, - /*ignoreChangeEvents*/ true, - /*ignoreDeleteEvents*/ true); - - watcher.onDidCreate(uri => { - watcher.dispose(); - resolve(); - }); - }).then(() => { - // 2nd try again - return this.autoStart(preferredPath); + const launchTargets = await findLaunchTargets(options); + + // If there aren't any potential launch targets, we create file watcher and try to + // start the server again once a *.sln, *.slnf, *.csproj, project.json, CSX or Cake file is created. + if (launchTargets.length === 0) { + await new Promise((resolve, reject) => { + // 1st watch for files + const watcher = this.vscode.workspace.createFileSystemWatcher('{**/*.sln,**/*.slnf,**/*.csproj,**/project.json,**/*.csx,**/*.cake}', + /*ignoreCreateEvents*/ false, + /*ignoreChangeEvents*/ true, + /*ignoreDeleteEvents*/ true); + + watcher.onDidCreate(uri => { + watcher.dispose(); + resolve(); }); - } - else if (launchTargets.length === 1) { - // If there's only one target, just start - return this.restart(launchTargets[0]); - } + }); - // First, try to launch against something that matches the user's preferred target - const defaultLaunchSolutionConfigValue = this.optionProvider.GetLatestOptions().defaultLaunchSolution; - const defaultLaunchSolutionTarget = launchTargets.find((a) => (path.basename(a.target) === defaultLaunchSolutionConfigValue)); - if (defaultLaunchSolutionTarget) { - return this.restart(defaultLaunchSolutionTarget); - } + // 2nd try again + return this.autoStart(preferredPath); + } + else if (launchTargets.length === 1) { + // If there's only one target, just start + return this._start(launchTargets[0]); + } - // If there's more than one launch target, we start the server if one of the targets - // matches the preferred path. - if (preferredPath.length > 0) { - const preferredLaunchTarget = launchTargets.find((a) => a.target === preferredPath); - if (preferredLaunchTarget) { - return this.restart(preferredLaunchTarget); - } - } + // First, try to launch against something that matches the user's preferred target + const defaultLaunchSolutionConfigValue = this.optionProvider.GetLatestOptions().defaultLaunchSolution; + const defaultLaunchSolutionTarget = launchTargets.find((a) => (path.basename(a.target) === defaultLaunchSolutionConfigValue)); + if (defaultLaunchSolutionTarget) { + return this._start(defaultLaunchSolutionTarget); + } - // To maintain previous behavior when there are mulitple targets available, - // launch with first Solution or Folder target. - const firstFolderOrSolutionTarget = launchTargets - .find(target => target.workspaceKind == LaunchTargetKind.Folder || target.workspaceKind == LaunchTargetKind.Solution); - if (firstFolderOrSolutionTarget) { - return this.restart(firstFolderOrSolutionTarget); + // If there's more than one launch target, we start the server if one of the targets + // matches the preferred path. + if (preferredPath.length > 0) { + const preferredLaunchTarget = launchTargets.find((a_1) => a_1.target === preferredPath); + if (preferredLaunchTarget) { + return this._start(preferredLaunchTarget); } + } - // When running integration tests, open the first launch target. - if (process.env.RUNNING_INTEGRATION_TESTS === "true") { - return this.restart(launchTargets[0]); - } + // To maintain previous behavior when there are mulitple targets available, + // launch with first Solution or Folder target. + const firstFolderOrSolutionTarget = launchTargets + .find(target => target.workspaceKind == LaunchTargetKind.Folder || target.workspaceKind == LaunchTargetKind.Solution); + if (firstFolderOrSolutionTarget) { + return this._start(firstFolderOrSolutionTarget); + } - // Otherwise, we fire the "MultipleLaunchTargets" event, - // which is handled in status.ts to display the launch target selector. - this._fireEvent(Events.MultipleLaunchTargets, launchTargets); - return showProjectSelector(this, launchTargets); - }); + // When running integration tests, open the first launch target. + if (process.env.RUNNING_INTEGRATION_TESTS === "true") { + return this._start(launchTargets[0]); + } + + // Otherwise, we fire the "MultipleLaunchTargets" event, + // which is handled in status.ts to display the launch target selector. + this._fireEvent(Events.MultipleLaunchTargets, launchTargets); + return showProjectSelector(this, launchTargets); } // --- requests et al public async makeRequest(command: string, data?: any, token?: CancellationToken): Promise { - if (!this.isRunning()) { return Promise.reject('OmniSharp server is not running.'); } @@ -620,7 +624,7 @@ export class OmniSharpServer { let startTime: number; let request: Request; - let promise = new Promise((resolve, reject) => { + const promise = new Promise((resolve, reject) => { startTime = Date.now(); request = { @@ -633,7 +637,7 @@ export class OmniSharpServer { this._requestQueue.enqueue(request); }); - if (token) { + if (token !== undefined) { token.onCancellationRequested(() => { this.eventStream.post(new ObservableEvents.OmnisharpServerRequestCancelled(request.command, request.id)); this._requestQueue.cancelRequest(request); @@ -651,18 +655,20 @@ export class OmniSharpServer { }); } - private async _doConnect(options: Options): Promise { - - this._serverProcess.stderr.on('data', (data: Buffer) => { + private async _doConnect( + disposables: CompositeDisposable, + serverProcess: ChildProcess, + options: Options): Promise { + serverProcess.stderr.on('data', (data: Buffer) => { let trimData = removeBOMFromBuffer(data); if (trimData.length > 0) { this._fireEvent(Events.StdErr, trimData.toString()); } }); - this._readLine = createInterface({ - input: this._serverProcess.stdout, - output: this._serverProcess.stdin, + const readLine = createInterface({ + input: serverProcess.stdout, + output: serverProcess.stdin, terminal: false }); @@ -694,10 +700,10 @@ export class OmniSharpServer { const lineReceived = this._onLineReceived.bind(this); - this._readLine.addListener('line', lineReceived); + readLine.addListener('line', lineReceived); - this._disposables.add(new Disposable(() => { - this._readLine.removeListener('line', lineReceived); + disposables.add(new Disposable(() => { + readLine.removeListener('line', lineReceived); })); return promise; @@ -769,7 +775,11 @@ export class OmniSharpServer { } } - private _makeRequest(request: Request) { + private _makeRequest(request: Request): number { + if (this._state.status !== ServerState.Started) { + throw new Error("Tried to make a request when the OmniSharp server wasn't running"); + } + const id = OmniSharpServer._nextId++; request.id = id; @@ -781,7 +791,7 @@ export class OmniSharpServer { }; this.eventStream.post(new ObservableEvents.OmnisharpRequestMessage(request, id)); - this._serverProcess.stdin.write(JSON.stringify(requestPacket) + '\n'); + this._state.serverProcess.stdin.write(JSON.stringify(requestPacket) + '\n'); return id; } }