Skip to content

Commit

Permalink
disable task reconnection by default (microsoft#156217)
Browse files Browse the repository at this point in the history
  • Loading branch information
meganrogge authored Jul 25, 2022
1 parent fbdc848 commit 364247d
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
processContext.set(process && !isVirtual);
}
this._onDidRegisterSupportedExecutions.fire();
if (this._jsonTasksSupported && !this._tasksReconnected) {
if (this._configurationService.getValue(TaskSettingId.Reconnection) === true && this._jsonTasksSupported && !this._tasksReconnected) {
this._reconnectTasks();
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/vs/workbench/contrib/tasks/browser/task.contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,12 @@ configurationRegistry.registerConfiguration({
description: nls.localize('task.showDecorations', "Shows decorations at points of interest in the terminal buffer such as the first problem found via a watch task. Note that this will only take effect for future tasks."),
default: true
},
[TaskSettingId.Reconnection]: {
type: 'boolean',
description: nls.localize('task.experimental.reconnection', "On window reload, reconnect to running watch/background tasks. Note that this is experimental, so you could encounter issues."),
default: false,
tags: ['experimental']
},
[TaskSettingId.SaveBeforeRun]: {
markdownDescription: nls.localize(
'task.saveBeforeRun',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,6 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
this._fireTaskEvent(TaskEvent.create(TaskEventKind.ProcessStarted, task, terminal!.processId!));
processStartedSignaled = true;
}

this._fireTaskEvent(TaskEvent.create(TaskEventKind.ProcessEnded, task, exitCode ?? undefined));

for (let i = 0; i < eventCounter; i++) {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/tasks/common/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,8 @@ export const enum TaskSettingId {
QuickOpenDetail = 'task.quickOpen.detail',
QuickOpenSkip = 'task.quickOpen.skip',
QuickOpenShowAll = 'task.quickOpen.showAll',
AllowAutomaticTasks = 'task.allowAutomaticTasks'
AllowAutomaticTasks = 'task.allowAutomaticTasks',
Reconnection = 'task.experimental.reconnection'
}

export const enum TasksSchemaProperties {
Expand Down
7 changes: 4 additions & 3 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import { IOpenerService } from 'vs/platform/opener/common/opener';
import { IGenericMarkProperties } from 'vs/platform/terminal/common/terminalProcess';
import { ICommandService } from 'vs/platform/commands/common/commands';
import { getIconRegistry } from 'vs/platform/theme/common/iconRegistry';
import { TaskSettingId } from 'vs/workbench/contrib/tasks/common/tasks';

const enum Constants {
/**
Expand Down Expand Up @@ -695,7 +696,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
this._shutdownPersistentProcessId = shutdownPersistentProcessId;
}
get persistentProcessId(): number | undefined { return this._processManager.persistentProcessId ?? this._shutdownPersistentProcessId; }
get shouldPersist(): boolean { return (this._processManager.shouldPersist || this._shutdownPersistentProcessId !== undefined) && !this.shellLaunchConfig.isTransient; }
get shouldPersist(): boolean { return (this._processManager.shouldPersist || this._shutdownPersistentProcessId !== undefined) && !this.shellLaunchConfig.isTransient && (!this.reconnectionOwner || this._configurationService.getValue(TaskSettingId.Reconnection) === true); }

/**
* Create xterm.js instance and attach data listeners.
Expand Down Expand Up @@ -2386,12 +2387,12 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}

// Recreate the process if the terminal has not yet been interacted with and it's not a
// special terminal (eg. task, extension terminal)
// special terminal (eg. extension terminal)
if (
info.requiresAction &&
this._configHelper.config.environmentChangesRelaunch &&
!this._processManager.hasWrittenData &&
(this.reconnectionOwner || !this._shellLaunchConfig.isFeatureTerminal) &&
(!this._shellLaunchConfig.isFeatureTerminal || (this.reconnectionOwner && this._configurationService.getValue(TaskSettingId.Reconnection) === true)) &&
!this._shellLaunchConfig.customPtyImplementation
&& !this._shellLaunchConfig.isExtensionOwnedTerminal &&
!this._shellLaunchConfig.attachPersistentProcess
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/
import { IHistoryService } from 'vs/workbench/services/history/common/history';
import { IPathService } from 'vs/workbench/services/path/common/pathService';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { TaskSettingId } from 'vs/workbench/contrib/tasks/common/tasks';

/** The amount of time to consider terminal errors to be related to the launch */
const LAUNCHING_DURATION = 500;
Expand Down Expand Up @@ -245,8 +246,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce

// this is a copy of what the merged environment collection is on the remote side
const env = await this._resolveEnvironment(backend, variableResolver, shellLaunchConfig);

const shouldPersist = (!!shellLaunchConfig.reconnectionOwner || !shellLaunchConfig.isFeatureTerminal) && this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.isTransient;
const shouldPersist = ((this._configurationService.getValue(TaskSettingId.Reconnection) && shellLaunchConfig.reconnectionOwner) || !shellLaunchConfig.isFeatureTerminal) && this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.isTransient;
if (shellLaunchConfig.attachPersistentProcess) {
const result = await backend.attachToProcess(shellLaunchConfig.attachPersistentProcess.id);
if (result) {
Expand Down Expand Up @@ -318,7 +318,6 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
}

this._process = newProcess;

this._setProcessState(ProcessState.Launching);

// Add any capabilities inherit to the backend
Expand Down Expand Up @@ -461,7 +460,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
windowsEnableConpty: this._configHelper.config.windowsEnableConpty && !isScreenReaderModeEnabled,
environmentVariableCollections: this._extEnvironmentVariableCollection ? serializeEnvironmentVariableCollections(this._extEnvironmentVariableCollection.collections) : undefined
};
const shouldPersist = this._configHelper.config.enablePersistentSessions && (!!this.reconnectionOwner || !shellLaunchConfig.isFeatureTerminal);
const shouldPersist = ((this._configurationService.getValue(TaskSettingId.Reconnection) && shellLaunchConfig.reconnectionOwner) || !shellLaunchConfig.isFeatureTerminal) && this._configHelper.config.enablePersistentSessions && !shellLaunchConfig.isTransient;
return await backend.createProcess(shellLaunchConfig, initialCwd, cols, rows, this._configHelper.config.unicodeVersion, env, options, shouldPersist);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ export class TerminalService implements ITerminalService {
// we update detected profiles when an instance is created so that,
// for example, we detect if you've installed a pwsh
this.onDidCreateInstance(() => this._terminalProfileService.refreshAvailableProfiles());

this._forwardInstanceHostEvents(this._terminalGroupService);
this._forwardInstanceHostEvents(this._terminalEditorService);
this._terminalGroupService.onDidChangeActiveGroup(this._onDidChangeActiveGroup.fire, this._onDidChangeActiveGroup);
Expand Down

0 comments on commit 364247d

Please sign in to comment.