From d91045dc5f31d14a7c0e3ce1fb66618a9baa75d6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 18:18:22 +0000 Subject: [PATCH 1/4] Cancel WriteBuffer innerWrite timers on dispose Guard _innerWrite and public write methods after disposal so pending setTimeout continuations cannot run against a disposed terminal. Fixes #5936 Co-authored-by: Daniel Imms --- src/common/input/WriteBuffer.test.ts | 29 +++++++++++++++ src/common/input/WriteBuffer.ts | 53 ++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/common/input/WriteBuffer.test.ts b/src/common/input/WriteBuffer.test.ts index cca6bd0be..5340f918a 100644 --- a/src/common/input/WriteBuffer.test.ts +++ b/src/common/input/WriteBuffer.test.ts @@ -140,5 +140,34 @@ describe('WriteBuffer', () => { wb.flushSync(); assert.equal(parsed, 0); }); + it('dispose cancels scheduled innerWrite', done => { + wb.write('a'); + wb.dispose(); + setTimeout(() => { + assert.deepEqual(stack, []); + done(); + }, 20); + }); + it('dispose does not fire onWriteParsed for pending writes', done => { + let parsed = 0; + wb.onWriteParsed(() => parsed++); + wb.write('a'); + wb.dispose(); + setTimeout(() => { + assert.equal(parsed, 0); + done(); + }, 20); + }); + it('write after dispose is a no-op', done => { + wb.dispose(); + wb.write('a', () => { + assert.deepEqual(stack, []); + done(); + }); + setTimeout(() => { + assert.deepEqual(stack, []); + done(); + }, 20); + }); }); }); diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 7c6d5d866..279dc7693 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -4,11 +4,10 @@ * @license MIT */ +import { TimeoutTimer } from 'common/Async'; import { Disposable } from 'common/Lifecycle'; import { Emitter } from 'common/Event'; -declare const setTimeout: (handler: () => void, timeout?: number) => void; - const enum Constants { /** * Safety watermark to avoid memory exhaustion and browser engine crash on fast data input. @@ -42,7 +41,9 @@ export class WriteBuffer extends Disposable { private _isSyncWriting = false; private _syncCalls = 0; private _didUserInput = false; + private _isDisposed = false; + private readonly _innerWriteTimer = this._register(new TimeoutTimer()); private readonly _onWriteParsed = this._register(new Emitter()); public readonly onWriteParsed = this._onWriteParsed.event; @@ -50,6 +51,18 @@ export class WriteBuffer extends Disposable { super(); } + public override dispose(): void { + if (this._isDisposed) { + return; + } + this._isDisposed = true; + this._writeBuffer.length = 0; + this._callbacks.length = 0; + this._pendingData = 0; + this._bufferOffset = 0; + super.dispose(); + } + public handleUserInput(): void { this._didUserInput = true; } @@ -63,6 +76,9 @@ export class WriteBuffer extends Disposable { * promises to resolve. */ public flushSync(): void { + if (this._isDisposed) { + return; + } // exit early if another sync write loop is active if (this._isSyncWriting) { return; @@ -95,6 +111,9 @@ export class WriteBuffer extends Disposable { * @deprecated Unreliable, to be removed soon. */ public writeSync(data: string | Uint8Array, maxSubsequentCalls?: number): void { + if (this._isDisposed) { + return; + } // stop writeSync recursions with maxSubsequentCalls argument // This is dangerous to use as it will lose the current data chunk // and return immediately. @@ -138,6 +157,9 @@ export class WriteBuffer extends Disposable { } public write(data: string | Uint8Array, callback?: () => void): void { + if (this._isDisposed) { + return; + } if (this._pendingData > Constants.DISCARD_WATERMARK) { throw new Error('write data discarded, use flow control to avoid losing data'); } @@ -158,7 +180,7 @@ export class WriteBuffer extends Disposable { return; } - setTimeout(() => this._innerWrite()); + this._scheduleInnerWrite(); } this._pendingData += data.length; @@ -194,7 +216,17 @@ export class WriteBuffer extends Disposable { * * Note, for pure sync code `lastTime` and `promiseResult` have no meaning. */ + private _scheduleInnerWrite(lastTime: number = 0, promiseResult: boolean = true): void { + if (this._isDisposed) { + return; + } + this._innerWriteTimer.cancelAndSet(() => this._innerWrite(lastTime, promiseResult), 0); + } + protected _innerWrite(lastTime: number = 0, promiseResult: boolean = true): void { + if (this._isDisposed) { + return; + } const startTime = lastTime || performance.now(); while (this._writeBuffer.length > this._bufferOffset) { const data = this._writeBuffer[this._bufferOffset]; @@ -223,9 +255,16 @@ export class WriteBuffer extends Disposable { * responsibility to slice hard work), but we can at least schedule a screen update as we * gain control. */ - const continuation: (r: boolean) => void = (r: boolean) => performance.now() - startTime >= Constants.WRITE_TIMEOUT_MS - ? setTimeout(() => this._innerWrite(0, r)) - : this._innerWrite(startTime, r); + const continuation: (r: boolean) => void = (r: boolean) => { + if (this._isDisposed) { + return; + } + if (performance.now() - startTime >= Constants.WRITE_TIMEOUT_MS) { + this._scheduleInnerWrite(0, r); + } else { + this._innerWrite(startTime, r); + } + }; /** * Optimization considerations: @@ -272,7 +311,7 @@ export class WriteBuffer extends Disposable { this._callbacks = this._callbacks.slice(this._bufferOffset); this._bufferOffset = 0; } - setTimeout(() => this._innerWrite()); + this._scheduleInnerWrite(); } else { this._writeBuffer.length = 0; this._callbacks.length = 0; From 7499a2181d90aeca5e473e9aa14ca6542d8c83ab Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 19:34:08 +0000 Subject: [PATCH 2/4] Use DisposableStore.isDisposed instead of duplicate flag Co-authored-by: Daniel Imms --- src/common/input/WriteBuffer.ts | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index 279dc7693..f606c8655 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -41,7 +41,6 @@ export class WriteBuffer extends Disposable { private _isSyncWriting = false; private _syncCalls = 0; private _didUserInput = false; - private _isDisposed = false; private readonly _innerWriteTimer = this._register(new TimeoutTimer()); private readonly _onWriteParsed = this._register(new Emitter()); @@ -52,10 +51,9 @@ export class WriteBuffer extends Disposable { } public override dispose(): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } - this._isDisposed = true; this._writeBuffer.length = 0; this._callbacks.length = 0; this._pendingData = 0; @@ -76,7 +74,7 @@ export class WriteBuffer extends Disposable { * promises to resolve. */ public flushSync(): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } // exit early if another sync write loop is active @@ -111,7 +109,7 @@ export class WriteBuffer extends Disposable { * @deprecated Unreliable, to be removed soon. */ public writeSync(data: string | Uint8Array, maxSubsequentCalls?: number): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } // stop writeSync recursions with maxSubsequentCalls argument @@ -157,7 +155,7 @@ export class WriteBuffer extends Disposable { } public write(data: string | Uint8Array, callback?: () => void): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } if (this._pendingData > Constants.DISCARD_WATERMARK) { @@ -217,14 +215,14 @@ export class WriteBuffer extends Disposable { * Note, for pure sync code `lastTime` and `promiseResult` have no meaning. */ private _scheduleInnerWrite(lastTime: number = 0, promiseResult: boolean = true): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } this._innerWriteTimer.cancelAndSet(() => this._innerWrite(lastTime, promiseResult), 0); } protected _innerWrite(lastTime: number = 0, promiseResult: boolean = true): void { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } const startTime = lastTime || performance.now(); @@ -256,7 +254,7 @@ export class WriteBuffer extends Disposable { * gain control. */ const continuation: (r: boolean) => void = (r: boolean) => { - if (this._isDisposed) { + if (this._store.isDisposed) { return; } if (performance.now() - startTime >= Constants.WRITE_TIMEOUT_MS) { From 3e0c7d18e50d89abf4c0523fc22cae6495d5b71d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 19:35:58 +0000 Subject: [PATCH 3/4] Register WriteBuffer cleanup via toDisposable in constructor Co-authored-by: Daniel Imms --- src/common/input/WriteBuffer.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/common/input/WriteBuffer.ts b/src/common/input/WriteBuffer.ts index f606c8655..7ecab58ad 100644 --- a/src/common/input/WriteBuffer.ts +++ b/src/common/input/WriteBuffer.ts @@ -5,7 +5,7 @@ */ import { TimeoutTimer } from 'common/Async'; -import { Disposable } from 'common/Lifecycle'; +import { Disposable, toDisposable } from 'common/Lifecycle'; import { Emitter } from 'common/Event'; const enum Constants { @@ -48,17 +48,12 @@ export class WriteBuffer extends Disposable { constructor(private _action: (data: string | Uint8Array, promiseResult?: boolean) => void | Promise) { super(); - } - - public override dispose(): void { - if (this._store.isDisposed) { - return; - } - this._writeBuffer.length = 0; - this._callbacks.length = 0; - this._pendingData = 0; - this._bufferOffset = 0; - super.dispose(); + this._register(toDisposable(() => { + this._writeBuffer.length = 0; + this._callbacks.length = 0; + this._pendingData = 0; + this._bufferOffset = 0; + })); } public handleUserInput(): void { From edb4f89dd8c86e04b5bd8289b3c66f2957603a00 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 20:00:06 +0000 Subject: [PATCH 4/4] Add WriteBuffer regression tests for dispose and hot paths Co-authored-by: Daniel Imms --- src/common/input/WriteBuffer.test.ts | 45 +++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/common/input/WriteBuffer.test.ts b/src/common/input/WriteBuffer.test.ts index 5340f918a..8d8c81fc2 100644 --- a/src/common/input/WriteBuffer.test.ts +++ b/src/common/input/WriteBuffer.test.ts @@ -160,14 +160,51 @@ describe('WriteBuffer', () => { }); it('write after dispose is a no-op', done => { wb.dispose(); - wb.write('a', () => { - assert.deepEqual(stack, []); - done(); - }); + wb.write('a'); setTimeout(() => { assert.deepEqual(stack, []); done(); }, 20); }); + it('dispose is idempotent', done => { + wb.write('a'); + wb.dispose(); + wb.dispose(); + setTimeout(() => { + assert.deepEqual(stack, []); + done(); + }, 20); + }); + it('async handler continuation is skipped after dispose', done => { + let resolve!: (value: boolean) => void; + const pending = new Promise(r => { resolve = r; }); + wb = new WriteBuffer(() => pending); + wb.write('a'); + wb.dispose(); + resolve(true); + setTimeout(() => { + assert.deepEqual(stack, []); + done(); + }, 20); + }); + it('handleUserInput still processes first chunk synchronously', () => { + wb.handleUserInput(); + wb.write('a'); + assert.deepEqual(stack, ['a']); + }); + it('flushSync after dispose is a no-op', done => { + wb.write('a'); + wb.dispose(); + wb.flushSync(); + setTimeout(() => { + assert.deepEqual(stack, []); + done(); + }, 20); + }); + it('writeSync after dispose is a no-op', () => { + wb.dispose(); + wb.writeSync('a'); + assert.deepEqual(stack, []); + }); }); });