Merge pull request #5950 from xtermjs/cursor/writebuffer-dispose-timeout-124c

Cancel WriteBuffer innerWrite timers on dispose

src/common/input/WriteBuffer.test.ts

@@ -140,5 +140,71 @@ 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');
+      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<boolean>(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, []);
+    });
   });
 });

src/common/input/WriteBuffer.ts

@@ -4,11 +4,10 @@
  * @license MIT
  */
 
-import { Disposable } from 'common/Lifecycle';
+import { TimeoutTimer } from 'common/Async';
+import { Disposable, toDisposable } 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.
@@ -43,11 +42,18 @@ export class WriteBuffer extends Disposable {
   private _syncCalls = 0;
   private _didUserInput = false;
 
+  private readonly _innerWriteTimer = this._register(new TimeoutTimer());
   private readonly _onWriteParsed = this._register(new Emitter<void>());
   public readonly onWriteParsed = this._onWriteParsed.event;
 
   constructor(private _action: (data: string | Uint8Array, promiseResult?: boolean) => void | Promise<boolean>) {
     super();
+    this._register(toDisposable(() => {
+      this._writeBuffer.length = 0;
+      this._callbacks.length = 0;
+      this._pendingData = 0;
+      this._bufferOffset = 0;
+    }));
   }
 
   public handleUserInput(): void {
@@ -63,6 +69,9 @@ export class WriteBuffer extends Disposable {
    * promises to resolve.
    */
   public flushSync(): void {
+    if (this._store.isDisposed) {
+      return;
+    }
     // exit early if another sync write loop is active
     if (this._isSyncWriting) {
       return;
@@ -95,6 +104,9 @@ export class WriteBuffer extends Disposable {
    * @deprecated Unreliable, to be removed soon.
    */
   public writeSync(data: string | Uint8Array, maxSubsequentCalls?: number): void {
+    if (this._store.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 +150,9 @@ export class WriteBuffer extends Disposable {
   }
 
   public write(data: string | Uint8Array, callback?: () => void): void {
+    if (this._store.isDisposed) {
+      return;
+    }
     if (this._pendingData > Constants.DISCARD_WATERMARK) {
       throw new Error('write data discarded, use flow control to avoid losing data');
     }
@@ -158,7 +173,7 @@ export class WriteBuffer extends Disposable {
         return;
       }
 
-      setTimeout(() => this._innerWrite());
+      this._scheduleInnerWrite();
     }
 
     this._pendingData += data.length;
@@ -194,7 +209,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._store.isDisposed) {
+      return;
+    }
+    this._innerWriteTimer.cancelAndSet(() => this._innerWrite(lastTime, promiseResult), 0);
+  }
+
   protected _innerWrite(lastTime: number = 0, promiseResult: boolean = true): void {
+    if (this._store.isDisposed) {
+      return;
+    }
     const startTime = lastTime || performance.now();
     while (this._writeBuffer.length > this._bufferOffset) {
       const data = this._writeBuffer[this._bufferOffset];
@@ -223,9 +248,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._store.isDisposed) {
+            return;
+          }
+          if (performance.now() - startTime >= Constants.WRITE_TIMEOUT_MS) {
+            this._scheduleInnerWrite(0, r);
+          } else {
+            this._innerWrite(startTime, r);
+          }
+        };
 
         /**
          * Optimization considerations:
@@ -272,7 +304,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;