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

Attempt to safetly prevent duplicated execution commandline #24416

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
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
Next Next commit
Start on partial keyboard interrupt fixes
Co-authored-by: Daniel Imms <[email protected]>
anthonykim1 committed Nov 11, 2024
commit c7a150471c62de9e6fab0914b34cd0a3b6c72309
34 changes: 27 additions & 7 deletions src/client/common/terminal/service.ts
Original file line number Diff line number Diff line change
@@ -2,17 +2,18 @@
// Licensed under the MIT License.

import { inject, injectable } from 'inversify';
import { CancellationToken, Disposable, Event, EventEmitter, Terminal, TerminalShellExecution } from 'vscode';
import { CancellationToken, Disposable, Event, EventEmitter, Terminal, window } from 'vscode';
import '../../common/extensions';
import { IInterpreterService } from '../../interpreter/contracts';
import { IServiceContainer } from '../../ioc/types';
import { captureTelemetry } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { ITerminalAutoActivation } from '../../terminals/types';
import { ICodeExecutionService, ITerminalAutoActivation } from '../../terminals/types';
import { ITerminalManager } from '../application/types';
import { _SCRIPTS_DIR } from '../process/internal/scripts/constants';
import { IConfigurationService, IDisposableRegistry } from '../types';
import {
IExecuteCommandResult,
ITerminalActivator,
ITerminalHelper,
ITerminalService,
@@ -57,14 +58,18 @@ export class TerminalService implements ITerminalService, Disposable {
});
}
}
public async sendCommand(command: string, args: string[], _?: CancellationToken): Promise<void> {
public async sendCommand(
command: string,
args: string[],
_?: CancellationToken,
): Promise<IExecuteCommandResult | undefined> {
await this.ensureTerminal();
const text = this.terminalHelper.buildCommandForTerminal(this.terminalShellType, command, args);
if (!this.options?.hideFromUser) {
this.terminal!.show(true);
}

await this.executeCommand(text);
return this.executeCommand(text);
}
/** @deprecated */
public async sendText(text: string): Promise<void> {
@@ -74,7 +79,7 @@ export class TerminalService implements ITerminalService, Disposable {
}
this.terminal!.sendText(text);
}
public async executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined> {
public async executeCommand(commandLine: string): Promise<IExecuteCommandResult | undefined> {
const terminal = this.terminal!;
if (!this.options?.hideFromUser) {
terminal.show(true);
@@ -97,11 +102,26 @@ export class TerminalService implements ITerminalService, Disposable {
});
await promise;
}

// python in a shell , exit code is undefined . startCommand event happen, we call end command event
if (terminal.shellIntegration) {
// TODO: Await the python REPL execute promise here. So we know python repl launched for sure before executing other python code.
// So we would not be interrupted.

await this.serviceContainer.get<ICodeExecutionService>(ICodeExecutionService).replActive;

const execution = terminal.shellIntegration.executeCommand(commandLine);
traceVerbose(`Shell Integration is enabled, executeCommand: ${commandLine}`);
return execution;
return {
execution,
exitCode: new Promise((resolve) => {
const disposable = window.onDidEndTerminalShellExecution((event) => {
if (event.execution === execution) {
disposable.dispose();
resolve(event.exitCode);
}
});
}),
};
} else {
terminal.sendText(commandLine);
traceVerbose(`Shell Integration is disabled, sendText: ${commandLine}`);
8 changes: 4 additions & 4 deletions src/client/common/terminal/syncTerminalService.ts
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@
'use strict';

import { inject } from 'inversify';
import { CancellationToken, Disposable, Event, TerminalShellExecution } from 'vscode';
import { CancellationToken, Disposable, Event } from 'vscode';
import { IInterpreterService } from '../../interpreter/contracts';
import { traceVerbose } from '../../logging';
import { PythonEnvironment } from '../../pythonEnvironments/info';
@@ -14,7 +14,7 @@ import * as internalScripts from '../process/internal/scripts';
import { createDeferred, Deferred } from '../utils/async';
import { noop } from '../utils/misc';
import { TerminalService } from './service';
import { ITerminalService } from './types';
import { IExecuteCommandResult, ITerminalService } from './types';

enum State {
notStarted = 0,
@@ -124,7 +124,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
args: string[],
cancel?: CancellationToken,
swallowExceptions: boolean = true,
): Promise<void> {
): Promise<IExecuteCommandResult | undefined> {
if (!cancel) {
return this.terminalService.sendCommand(command, args);
}
@@ -145,7 +145,7 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
public sendText(text: string): Promise<void> {
return this.terminalService.sendText(text);
}
public executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined> {
public async executeCommand(commandLine: string): Promise<IExecuteCommandResult | undefined> {
return this.terminalService.executeCommand(commandLine);
}
public show(preserveFocus?: boolean | undefined): Promise<void> {
9 changes: 7 additions & 2 deletions src/client/common/terminal/types.ts
Original file line number Diff line number Diff line change
@@ -51,15 +51,20 @@ export interface ITerminalService extends IDisposable {
args: string[],
cancel?: CancellationToken,
swallowExceptions?: boolean,
): Promise<void>;
): Promise<IExecuteCommandResult | undefined>;
/** @deprecated */
sendText(text: string): Promise<void>;
executeCommand(commandLine: string): Promise<TerminalShellExecution | undefined>;
executeCommand(commandLine: string): Promise<IExecuteCommandResult | undefined>;
show(preserveFocus?: boolean): Promise<void>;
}

export const ITerminalServiceFactory = Symbol('ITerminalServiceFactory');

export interface IExecuteCommandResult {
execution: TerminalShellExecution;
exitCode: Promise<number | undefined>;
}

export type TerminalCreationOptions = {
/**
* Object with environment variables that will be added to the Terminal.
13 changes: 11 additions & 2 deletions src/client/terminals/codeExecution/terminalCodeExecution.ts
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ import { sendTelemetryEvent } from '../../telemetry';
export class TerminalCodeExecutionProvider implements ICodeExecutionService {
private hasRanOutsideCurrentDrive = false;
protected terminalTitle!: string;
private replActive?: Promise<boolean>;
replActive?: Promise<boolean>;

constructor(
@inject(ITerminalServiceFactory) protected readonly terminalServiceFactory: ITerminalServiceFactory,
@@ -73,6 +73,7 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
this.replActive = new Promise<boolean>(async (resolve) => {
const replCommandArgs = await this.getExecutableInfo(resource);
let listener: IDisposable;
// TODO: There's a race condition here as well; the send text from file command may happen before this timeout resolves
Promise.race([
new Promise<boolean>((resolve) => setTimeout(() => resolve(true), 3000)),
new Promise<boolean>((resolve) => {
@@ -99,9 +100,17 @@ export class TerminalCodeExecutionProvider implements ICodeExecutionService {
}
resolve(true);
});
// python
// python shell, exit code undefined.
// ondidStartShellExecution happens, si fires ondidEndsi

await terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args);
// TODO: Store this promise and make sure no further commands are executed without awaiting .exitCode
const replExecution = await terminalService.sendCommand(replCommandArgs.command, replCommandArgs.args); // need to make sure this is resolved before starting executing something in repl.

// TODO: Should we await replyExecution.exitCode here?
await replExecution?.exitCode;
});

this.disposables.push(
terminalService.onDidCloseTerminal(() => {
this.replActive = undefined;
1 change: 1 addition & 0 deletions src/client/terminals/types.ts
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ import { IDisposable, Resource } from '../common/types';
export const ICodeExecutionService = Symbol('ICodeExecutionService');

export interface ICodeExecutionService {
replActive?: Promise<boolean>;
execute(code: string, resource?: Uri): Promise<void>;
executeFile(file: Uri, options?: { newTerminalPerFile: boolean }): Promise<void>;
initializeRepl(resource?: Uri): Promise<void>;