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 <Tyriar@users.noreply.github.com>

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);
+    });
   });
 });

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<void>());
   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;