Skip to content

Commit 57c4500

Browse files
TyriarTyriar
authored
Merge pull request microsoft#87999 from microsoft/tyriar/85257
Flush all buffered data events on terminal close
2 parents 1b17e36 + 027d7ca commit 57c4500

5 files changed

Lines changed: 136 additions & 108 deletions

File tree

‎extensions/vscode-api-tests/src/singlefolder-tests/workspace.tasks.test.ts‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,64 @@ suite('workspace-namespace', () => {
6969
},
7070
resolveTask(_task: Task): Task | undefined {
7171
try {
72-
assert.fail('resolveTask should not trigger during the test');
72+
assert.fail('resolveTask should not trigger during the test');
73+
} catch (e) {
74+
done(e);
75+
}
76+
return undefined;
77+
}
78+
}));
79+
commands.executeCommand('workbench.action.tasks.runTask', `${taskType}: ${taskName}`);
80+
});
81+
82+
test('sync CustomExecution task should flush all data on close', (done) => {
83+
interface CustomTestingTaskDefinition extends TaskDefinition {
84+
/**
85+
* One of the task properties. This can be used to customize the task in the tasks.json
86+
*/
87+
customProp1: string;
88+
}
89+
const taskType: string = 'customTesting';
90+
const taskName = 'First custom task';
91+
disposables.push(window.onDidOpenTerminal(term => {
92+
disposables.push(window.onDidWriteTerminalData(e => {
93+
try {
94+
assert.equal(e.data, 'exiting');
95+
} catch (e) {
96+
done(e);
97+
}
98+
disposables.push(window.onDidCloseTerminal(() => done()));
99+
term.dispose();
100+
}));
101+
}));
102+
disposables.push(tasks.registerTaskProvider(taskType, {
103+
provideTasks: () => {
104+
const result: Task[] = [];
105+
const kind: CustomTestingTaskDefinition = {
106+
type: taskType,
107+
customProp1: 'testing task one'
108+
};
109+
const writeEmitter = new EventEmitter<string>();
110+
const closeEmitter = new EventEmitter<void>();
111+
const execution = new CustomExecution((): Thenable<Pseudoterminal> => {
112+
const pty: Pseudoterminal = {
113+
onDidWrite: writeEmitter.event,
114+
onDidClose: closeEmitter.event,
115+
open: () => {
116+
writeEmitter.fire('exiting');
117+
closeEmitter.fire();
118+
},
119+
close: () => {}
120+
};
121+
return Promise.resolve(pty);
122+
});
123+
const task = new Task2(kind, TaskScope.Workspace, taskName, taskType, execution);
124+
result.push(task);
125+
return result;
126+
},
127+
resolveTask(_task: Task): Task | undefined {
128+
try {
129+
assert.fail('resolveTask should not trigger during the test');
73130
} catch (e) {
74131
done(e);
75132
}

‎src/vs/workbench/api/browser/mainThreadTerminalService.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ class TerminalDataEventTracker extends Disposable {
343343
) {
344344
super();
345345

346-
this._register(this._bufferer = new TerminalDataBufferer());
346+
this._register(this._bufferer = new TerminalDataBufferer(this._callback));
347347

348348
this._terminalService.terminalInstances.forEach(instance => this._registerInstance(instance));
349349
this._register(this._terminalService.onInstanceCreated(instance => this._registerInstance(instance)));
@@ -352,6 +352,6 @@ class TerminalDataEventTracker extends Disposable {
352352

353353
private _registerInstance(instance: ITerminalInstance): void {
354354
// Buffer data events to reduce the amount of messages going to the extension host
355-
this._register(this._bufferer.startBuffering(instance.id, instance.onData, this._callback));
355+
this._register(this._bufferer.startBuffering(instance.id, instance.onData));
356356
}
357357
}

‎src/vs/workbench/api/common/extHostTerminalService.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,8 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
310310
constructor(
311311
@IExtHostRpcService extHostRpc: IExtHostRpcService
312312
) {
313-
this._bufferer = new TerminalDataBufferer();
314-
315313
this._proxy = extHostRpc.getProxy(MainContext.MainThreadTerminalService);
314+
this._bufferer = new TerminalDataBufferer(this._proxy.$sendProcessData);
316315
this._onDidWriteTerminalData = new Emitter<vscode.TerminalDataWriteEvent>({
317316
onFirstListenerAdd: () => this._proxy.$startSendingDataEvents(),
318317
onLastListenerRemove: () => this._proxy.$stopSendingDataEvents()
@@ -477,7 +476,7 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
477476
p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title));
478477

479478
// Buffer data events to reduce the amount of messages going to the renderer
480-
this._bufferer.startBuffering(id, p.onProcessData, this._proxy.$sendProcessData);
479+
this._bufferer.startBuffering(id, p.onProcessData);
481480
p.onProcessExit(exitCode => this._onProcessExit(id, exitCode));
482481

483482
if (p.onProcessOverrideDimensions) {

‎src/vs/workbench/contrib/terminal/common/terminalDataBuffering.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ interface TerminalDataBuffer extends IDisposable {
1414
export class TerminalDataBufferer implements IDisposable {
1515
private readonly _terminalBufferMap = new Map<number, TerminalDataBuffer>();
1616

17+
constructor(private readonly _callback: (id: number, data: string) => void) {
18+
}
19+
1720
dispose() {
1821
for (const buffer of this._terminalBufferMap.values()) {
1922
buffer.dispose();
2023
}
2124
}
2225

23-
startBuffering(id: number, event: Event<string>, callback: (id: number, data: string) => void, throttleBy: number = 5): IDisposable {
26+
startBuffering(id: number, event: Event<string>, throttleBy: number = 5): IDisposable {
2427
let disposable: IDisposable;
2528
disposable = event((e: string) => {
2629
let buffer = this._terminalBufferMap.get(id);
@@ -30,16 +33,13 @@ export class TerminalDataBufferer implements IDisposable {
3033
return;
3134
}
3235

33-
const timeoutId = setTimeout(() => {
34-
this._terminalBufferMap.delete(id);
35-
callback(id, buffer!.data.join(''));
36-
}, throttleBy);
36+
const timeoutId = setTimeout(() => this._flushBuffer(id), throttleBy);
3737
buffer = {
3838
data: [e],
3939
timeoutId: timeoutId,
4040
dispose: () => {
4141
clearTimeout(timeoutId);
42-
this._terminalBufferMap.delete(id);
42+
this._flushBuffer(id);
4343
disposable.dispose();
4444
}
4545
};
@@ -54,4 +54,12 @@ export class TerminalDataBufferer implements IDisposable {
5454
buffer.dispose();
5555
}
5656
}
57+
58+
private _flushBuffer(id: number): void {
59+
const buffer = this._terminalBufferMap.get(id);
60+
if (buffer) {
61+
this._terminalBufferMap.delete(id);
62+
this._callback(id, buffer.data.join(''));
63+
}
64+
}
5765
}

‎src/vs/workbench/contrib/terminal/test/common/terminalDataBuffering.test.ts‎

Lines changed: 60 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,28 @@ const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms));
1111

1212
suite('Workbench - TerminalDataBufferer', () => {
1313
let bufferer: TerminalDataBufferer;
14+
let counter: { [id: number]: number };
15+
let data: { [id: number]: string };
1416

1517
setup(async () => {
16-
bufferer = new TerminalDataBufferer();
18+
counter = {};
19+
data = {};
20+
bufferer = new TerminalDataBufferer((id, e) => {
21+
if (!(id in counter)) {
22+
counter[id] = 0;
23+
}
24+
counter[id]++;
25+
if (!(id in data)) {
26+
data[id] = '';
27+
}
28+
data[id] = e;
29+
});
1730
});
1831

1932
test('start', async () => {
20-
let terminalOnData = new Emitter<string>();
21-
let counter = 0;
22-
let data: string | undefined;
33+
const terminalOnData = new Emitter<string>();
2334

24-
bufferer.startBuffering(1, terminalOnData.event, (id, e) => {
25-
counter++;
26-
data = e;
27-
}, 0);
35+
bufferer.startBuffering(1, terminalOnData.event, 0);
2836

2937
terminalOnData.fire('1');
3038
terminalOnData.fire('2');
@@ -34,33 +42,21 @@ suite('Workbench - TerminalDataBufferer', () => {
3442

3543
terminalOnData.fire('4');
3644

37-
assert.equal(counter, 1);
38-
assert.equal(data, '123');
45+
assert.equal(counter[1], 1);
46+
assert.equal(data[1], '123');
3947

4048
await wait(0);
4149

42-
assert.equal(counter, 2);
43-
assert.equal(data, '4');
50+
assert.equal(counter[1], 2);
51+
assert.equal(data[1], '4');
4452
});
4553

4654
test('start 2', async () => {
47-
let terminal1OnData = new Emitter<string>();
48-
let terminal1Counter = 0;
49-
let terminal1Data: string | undefined;
50-
51-
bufferer.startBuffering(1, terminal1OnData.event, (id, e) => {
52-
terminal1Counter++;
53-
terminal1Data = e;
54-
}, 0);
55-
56-
let terminal2OnData = new Emitter<string>();
57-
let terminal2Counter = 0;
58-
let terminal2Data: string | undefined;
55+
const terminal1OnData = new Emitter<string>();
56+
const terminal2OnData = new Emitter<string>();
5957

60-
bufferer.startBuffering(2, terminal2OnData.event, (id, e) => {
61-
terminal2Counter++;
62-
terminal2Data = e;
63-
}, 0);
58+
bufferer.startBuffering(1, terminal1OnData.event, 0);
59+
bufferer.startBuffering(2, terminal2OnData.event, 0);
6460

6561
terminal1OnData.fire('1');
6662
terminal2OnData.fire('4');
@@ -70,60 +66,41 @@ suite('Workbench - TerminalDataBufferer', () => {
7066
terminal2OnData.fire('6');
7167
terminal2OnData.fire('7');
7268

73-
assert.equal(terminal1Counter, 0);
74-
assert.equal(terminal1Data, undefined);
75-
assert.equal(terminal2Counter, 0);
76-
assert.equal(terminal2Data, undefined);
69+
assert.equal(counter[1], undefined);
70+
assert.equal(data[1], undefined);
71+
assert.equal(counter[2], undefined);
72+
assert.equal(data[2], undefined);
7773

7874
await wait(0);
7975

80-
assert.equal(terminal1Counter, 1);
81-
assert.equal(terminal1Data, '123');
82-
assert.equal(terminal2Counter, 1);
83-
assert.equal(terminal2Data, '4567');
76+
assert.equal(counter[1], 1);
77+
assert.equal(data[1], '123');
78+
assert.equal(counter[2], 1);
79+
assert.equal(data[2], '4567');
8480
});
8581

8682
test('stop', async () => {
8783
let terminalOnData = new Emitter<string>();
88-
let counter = 0;
89-
let data: string | undefined;
9084

91-
bufferer.startBuffering(1, terminalOnData.event, (id, e) => {
92-
counter++;
93-
data = e;
94-
}, 0);
85+
bufferer.startBuffering(1, terminalOnData.event, 0);
9586

9687
terminalOnData.fire('1');
9788
terminalOnData.fire('2');
9889
terminalOnData.fire('3');
9990

10091
bufferer.stopBuffering(1);
101-
10292
await wait(0);
10393

104-
assert.equal(counter, 0);
105-
assert.equal(data, undefined);
94+
assert.equal(counter[1], 1);
95+
assert.equal(data[1], '123');
10696
});
10797

10898
test('start 2 stop 1', async () => {
109-
let terminal1OnData = new Emitter<string>();
110-
let terminal1Counter = 0;
111-
let terminal1Data: string | undefined;
112-
113-
bufferer.startBuffering(1, terminal1OnData.event, (id, e) => {
114-
terminal1Counter++;
115-
terminal1Data = e;
116-
}, 0);
117-
118-
let terminal2OnData = new Emitter<string>();
119-
let terminal2Counter = 0;
120-
let terminal2Data: string | undefined;
121-
122-
bufferer.startBuffering(2, terminal2OnData.event, (id, e) => {
123-
terminal2Counter++;
124-
terminal2Data = e;
125-
}, 0);
99+
const terminal1OnData = new Emitter<string>();
100+
const terminal2OnData = new Emitter<string>();
126101

102+
bufferer.startBuffering(1, terminal1OnData.event, 0);
103+
bufferer.startBuffering(2, terminal2OnData.event, 0);
127104

128105
terminal1OnData.fire('1');
129106
terminal2OnData.fire('4');
@@ -133,39 +110,26 @@ suite('Workbench - TerminalDataBufferer', () => {
133110
terminal2OnData.fire('6');
134111
terminal2OnData.fire('7');
135112

136-
assert.equal(terminal1Counter, 0);
137-
assert.equal(terminal1Data, undefined);
138-
assert.equal(terminal2Counter, 0);
139-
assert.equal(terminal2Data, undefined);
113+
assert.equal(counter[1], undefined);
114+
assert.equal(data[1], undefined);
115+
assert.equal(counter[2], undefined);
116+
assert.equal(data[2], undefined);
140117

141118
bufferer.stopBuffering(1);
142119
await wait(0);
143120

144-
assert.equal(terminal1Counter, 0);
145-
assert.equal(terminal1Data, undefined);
146-
assert.equal(terminal2Counter, 1);
147-
assert.equal(terminal2Data, '4567');
121+
assert.equal(counter[1], 1);
122+
assert.equal(data[1], '123');
123+
assert.equal(counter[2], 1);
124+
assert.equal(data[2], '4567');
148125
});
149126

150-
test('dispose', async () => {
151-
let terminal1OnData = new Emitter<string>();
152-
let terminal1Counter = 0;
153-
let terminal1Data: string | undefined;
154-
155-
bufferer.startBuffering(1, terminal1OnData.event, (id, e) => {
156-
terminal1Counter++;
157-
terminal1Data = e;
158-
}, 0);
159-
160-
let terminal2OnData = new Emitter<string>();
161-
let terminal2Counter = 0;
162-
let terminal2Data: string | undefined;
163-
164-
bufferer.startBuffering(2, terminal2OnData.event, (id, e) => {
165-
terminal2Counter++;
166-
terminal2Data = e;
167-
}, 0);
127+
test('dispose should flush remaining data events', async () => {
128+
const terminal1OnData = new Emitter<string>();
129+
const terminal2OnData = new Emitter<string>();
168130

131+
bufferer.startBuffering(1, terminal1OnData.event, 0);
132+
bufferer.startBuffering(2, terminal2OnData.event, 0);
169133

170134
terminal1OnData.fire('1');
171135
terminal2OnData.fire('4');
@@ -175,17 +139,17 @@ suite('Workbench - TerminalDataBufferer', () => {
175139
terminal2OnData.fire('6');
176140
terminal2OnData.fire('7');
177141

178-
assert.equal(terminal1Counter, 0);
179-
assert.equal(terminal1Data, undefined);
180-
assert.equal(terminal2Counter, 0);
181-
assert.equal(terminal2Data, undefined);
142+
assert.equal(counter[1], undefined);
143+
assert.equal(data[1], undefined);
144+
assert.equal(counter[2], undefined);
145+
assert.equal(data[2], undefined);
182146

183147
bufferer.dispose();
184148
await wait(0);
185149

186-
assert.equal(terminal1Counter, 0);
187-
assert.equal(terminal1Data, undefined);
188-
assert.equal(terminal2Counter, 0);
189-
assert.equal(terminal2Data, undefined);
150+
assert.equal(counter[1], 1);
151+
assert.equal(data[1], '123');
152+
assert.equal(counter[2], 1);
153+
assert.equal(data[2], '4567');
190154
});
191155
});

0 commit comments

Comments
 (0)