Skip to content

Commit 7651bec

Browse files
committed
eng: reapply test leak fixes
Re-applies #190623 which I merged before everyone had a chance to review. Alex and Ben, you were the two people whose code this touches and didn't review it in the original PR.
1 parent 5d0b606 commit 7651bec

File tree

33 files changed

+124
-80
lines changed

33 files changed

+124
-80
lines changed

src/vs/base/browser/ui/grid/gridview.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,7 @@ class BranchNode implements ISplitView<ILayoutContext>, IDisposable {
705705
this.splitviewSashResetDisposable.dispose();
706706
this.childrenSashResetDisposable.dispose();
707707
this.childrenChangeDisposable.dispose();
708+
this.onDidScrollDisposable.dispose();
708709
this.splitview.dispose();
709710
}
710711
}

src/vs/base/browser/ui/splitview/splitview.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,11 +565,11 @@ export class SplitView<TLayoutContext = undefined> extends Disposable {
565565
this.sashContainer = append(this.el, $('.sash-container'));
566566
this.viewContainer = $('.split-view-container');
567567

568-
this.scrollable = new Scrollable({
568+
this.scrollable = this._register(new Scrollable({
569569
forceIntegerValues: true,
570570
smoothScrollDuration: 125,
571571
scheduleAtNextAnimationFrame
572-
});
572+
}));
573573
this.scrollableElement = this._register(new SmoothScrollableElement(this.viewContainer, {
574574
vertical: this.orientation === Orientation.VERTICAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden,
575575
horizontal: this.orientation === Orientation.HORIZONTAL ? (options.scrollbarVisibility ?? ScrollbarVisibility.Auto) : ScrollbarVisibility.Hidden

src/vs/base/browser/ui/toolbar/toolbar.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ export class ToolBar extends Disposable {
212212

213213
override dispose(): void {
214214
this.clear();
215+
this.disposables.dispose();
215216
super.dispose();
216217
}
217218
}

src/vs/base/common/stream.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -519,20 +519,22 @@ export function consumeStream<T, R = T>(stream: ReadableStreamEvents<T>, reducer
519519
return new Promise((resolve, reject) => {
520520
const chunks: T[] = [];
521521

522-
listenStream(stream, {
522+
const l = listenStream(stream, {
523523
onData: chunk => {
524524
if (reducer) {
525525
chunks.push(chunk);
526526
}
527527
},
528528
onError: error => {
529+
l.dispose();
529530
if (reducer) {
530531
reject(error);
531532
} else {
532533
resolve(undefined);
533534
}
534535
},
535536
onEnd: () => {
537+
l.dispose();
536538
if (reducer) {
537539
resolve(reducer(chunks));
538540
} else {
@@ -570,15 +572,18 @@ export interface IStreamListener<T> {
570572
export function listenStream<T>(stream: ReadableStreamEvents<T>, listener: IStreamListener<T>): IDisposable {
571573
let destroyed = false;
572574

575+
// error and end events are in the next microtask so that a stream that is
576+
// closed synchronously (e.g from a memory `toStream`) can get its disposable
577+
// and destroy the stream.
573578
stream.on('error', error => {
574579
if (!destroyed) {
575-
listener.onError(error);
580+
queueMicrotask(() => listener.onError(error));
576581
}
577582
});
578583

579584
stream.on('end', () => {
580585
if (!destroyed) {
581-
listener.onEnd();
586+
queueMicrotask(() => listener.onEnd());
582587
}
583588
});
584589

@@ -692,10 +697,16 @@ export function toReadable<T>(t: T): Readable<T> {
692697
export function transform<Original, Transformed>(stream: ReadableStreamEvents<Original>, transformer: ITransformer<Original, Transformed>, reducer: IReducer<Transformed>): ReadableStream<Transformed> {
693698
const target = newWriteableStream<Transformed>(reducer);
694699

695-
listenStream(stream, {
700+
const l = listenStream(stream, {
696701
onData: data => target.write(transformer.data(data)),
697-
onError: error => target.error(transformer.error ? transformer.error(error) : error),
698-
onEnd: () => target.end()
702+
onError: error => {
703+
l.dispose();
704+
target.error(transformer.error ? transformer.error(error) : error);
705+
},
706+
onEnd: () => {
707+
l.dispose();
708+
target.end();
709+
}
699710
});
700711

701712
return target;
@@ -740,7 +751,7 @@ export function prefixedStream<T>(prefix: T, stream: ReadableStream<T>, reducer:
740751

741752
const target = newWriteableStream<T>(reducer);
742753

743-
listenStream(stream, {
754+
const l = listenStream(stream, {
744755
onData: data => {
745756

746757
// Handle prefix only once
@@ -752,8 +763,12 @@ export function prefixedStream<T>(prefix: T, stream: ReadableStream<T>, reducer:
752763

753764
return target.write(data);
754765
},
755-
onError: error => target.error(error),
766+
onError: error => {
767+
l.dispose();
768+
target.error(error);
769+
},
756770
onEnd: () => {
771+
l.dispose();
757772

758773
// Handle prefix only once
759774
if (!prefixHandled) {

src/vs/base/test/common/stream.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,14 +315,14 @@ suite('Stream', () => {
315315
assert.strictEqual(consumed, undefined);
316316
});
317317

318-
test('listenStream', () => {
318+
test('listenStream', async () => {
319319
const stream = newWriteableStream<string>(strings => strings.join());
320320

321321
let error = false;
322322
let end = false;
323323
let data = '';
324324

325-
listenStream(stream, {
325+
const l = listenStream(stream, {
326326
onData: d => {
327327
data = d;
328328
},
@@ -345,10 +345,14 @@ suite('Stream', () => {
345345
assert.strictEqual(end, false);
346346

347347
stream.error(new Error());
348+
await new Promise<void>(r => queueMicrotask(r));
348349
assert.strictEqual(error, true);
349350

350351
stream.end('Final Bit');
352+
await new Promise<void>(r => queueMicrotask(r));
351353
assert.strictEqual(end, true);
354+
355+
l.dispose();
352356
});
353357

354358
test('listenStream - dispose', () => {

src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ export class DiffEditorEditors extends Disposable {
3535
) {
3636
super();
3737

38-
this.original = this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {});
39-
this.modified = this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {});
38+
this.original = this._register(this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {}));
39+
this.modified = this._register(this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {}));
4040

4141
this._register(autorunHandleChanges({
4242
createEmptyChangeSummary: () => ({} as IDiffEditorConstructionOptions),

src/vs/editor/common/model/textModel.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,21 @@ export function createTextBufferFactoryFromStream(stream: ITextStream | VSBuffer
6767

6868
let done = false;
6969

70-
listenStream<string | VSBuffer>(stream, {
70+
const l = listenStream<string | VSBuffer>(stream, {
7171
onData: chunk => {
7272
builder.acceptChunk((typeof chunk === 'string') ? chunk : chunk.toString());
7373
},
7474
onError: error => {
7575
if (!done) {
7676
done = true;
77+
l.dispose();
7778
reject(error);
7879
}
7980
},
8081
onEnd: () => {
8182
if (!done) {
8283
done = true;
84+
l.dispose();
8385
resolve(builder.finish());
8486
}
8587
}

src/vs/editor/contrib/suggest/browser/suggestController.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export class SuggestController implements IEditorContribution {
143143
// context key: update insert/replace mode
144144
const ctxInsertMode = SuggestContext.InsertMode.bindTo(_contextKeyService);
145145
ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode);
146-
this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode));
146+
this._toDispose.add(this.model.onDidTrigger(() => ctxInsertMode.set(editor.getOption(EditorOption.suggest).insertMode)));
147147

148148
this.widget = this._toDispose.add(new IdleValue(() => {
149149

src/vs/platform/actions/browser/buttonbar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class MenuWorkbenchButtonBar extends ButtonBar {
5555
'workbenchActionExecuted',
5656
{ id: e.action.id, from: options.telemetrySource! }
5757
);
58-
}, this._store);
58+
}, undefined, this._store);
5959
}
6060

6161
const conifgProvider: IButtonConfigProvider = options?.buttonConfigProvider ?? (() => ({ showLabel: true }));

src/vs/platform/checksum/node/checksumService.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,16 @@ export class ChecksumService implements IChecksumService {
2020
return new Promise<string>((resolve, reject) => {
2121
const hash = createHash('md5');
2222

23-
listenStream(stream, {
23+
const l = listenStream(stream, {
2424
onData: data => hash.update(data.buffer),
25-
onError: error => reject(error),
26-
onEnd: () => resolve(hash.digest('base64').replace(/=+$/, ''))
25+
onError: error => {
26+
l.dispose();
27+
reject(error);
28+
},
29+
onEnd: () => {
30+
l.dispose();
31+
resolve(hash.digest('base64').replace(/=+$/, ''));
32+
}
2733
});
2834
});
2935
}

0 commit comments

Comments
 (0)