Cache/buffer lifecycle

src/common/buffer/Buffer.test.ts

@@ -1192,18 +1192,18 @@ describe('Buffer', () => {
 
       const cache = (buffer as any)._stringCache;
       assert.equal(cache.entries.size, 2);
-      const previousTimer = (buffer as any)._stringCacheClearTimeout;
+      const previousTimer = (cache as any)._clearTimeout;
       assert.notEqual(previousTimer, undefined);
 
       // Another translation should refresh the same shared timer, not create a second queue.
       assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`);
-      assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined);
-      assert.notEqual((buffer as any)._stringCacheClearTimeout, previousTimer);
+      assert.notEqual((cache as any)._clearTimeout, undefined);
+      assert.notEqual((cache as any)._clearTimeout, previousTimer);
 
       await new Promise(r => setTimeout(r, 70));
 
       assert.equal(cache.entries.size, 0);
-      assert.equal((buffer as any)._stringCacheClearTimeout, undefined);
+      assert.equal((cache as any)._clearTimeout, undefined);
 
       // Cache entries should be recreated lazily after a timer-based reset.
       assert.equal(buffer.translateBufferLineToString(0, false), `a${' '.repeat(INIT_COLS - 1)}`);
@@ -1217,11 +1217,11 @@ describe('Buffer', () => {
 
       const cache = (buffer as any)._stringCache;
       assert.equal(cache.entries.size, 1);
-      assert.notEqual((buffer as any)._stringCacheClearTimeout, undefined);
+      assert.notEqual((cache as any)._clearTimeout, undefined);
 
       buffer.clear();
       assert.equal(cache.entries.size, 0);
-      assert.equal((buffer as any)._stringCacheClearTimeout, undefined);
+      assert.equal((cache as any)._clearTimeout, undefined);
 
       buffer.fillViewportRows();
       buffer.lines.get(0)!.setCell(0, createCellData(0, 'b', 1));
@@ -1230,7 +1230,7 @@ describe('Buffer', () => {
 
       buffer.resize(INIT_COLS - 1, INIT_ROWS);
       assert.equal(cache.entries.size, 0);
-      assert.equal((buffer as any)._stringCacheClearTimeout, undefined);
+      assert.equal((cache as any)._clearTimeout, undefined);
     });
   });
 

src/common/buffer/Buffer.ts

@@ -4,6 +4,7 @@
  */
 
 import { CircularList, IInsertEvent } from 'common/CircularList';
+import { Disposable, toDisposable } from 'common/Lifecycle';
 import { IdleTaskQueue } from 'common/TaskQueue';
 import { IAttributeData, IBufferLine, ICellData, ICharset } from 'common/Types';
 import { ExtendedAttrs } from 'common/buffer/AttributeData';
@@ -18,7 +19,6 @@ import { DEFAULT_CHARSET } from 'common/data/Charsets';
 import { IBufferService, ILogService, IOptionsService } from 'common/services/Services';
 
 export const MAX_BUFFER_SIZE = 4294967295; // 2^32 - 1
-const STRING_CACHE_CLEAR_DELAY_MS = 50;
 
 /**
  * This class represents a terminal buffer (an internal state of the terminal), where the
@@ -27,7 +27,7 @@ const STRING_CACHE_CLEAR_DELAY_MS = 50;
  *   - cursor position
  *   - scroll position
  */
-export class Buffer implements IBuffer {
+export class Buffer extends Disposable implements IBuffer {
   public lines: CircularList<IBufferLine>;
   public ydisp: number = 0;
   public ybase: number = 0;
@@ -53,7 +53,6 @@ export class Buffer implements IBuffer {
   private _memoryCleanupQueue: InstanceType<typeof IdleTaskQueue>;
   private _memoryCleanupPosition = 0;
   private readonly _stringCache: BufferLineStringCache;
-  private _stringCacheClearTimeout: ReturnType<typeof setTimeout> | undefined;
 
   constructor(
     private _hasScrollback: boolean,
@@ -61,6 +60,8 @@ export class Buffer implements IBuffer {
     private _bufferService: IBufferService,
     private readonly _logService: ILogService
   ) {
+    super();
+    this._register(toDisposable(() => this.clearAllMarkers()));
     this._cols = this._bufferService.cols;
     this._rows = this._bufferService.rows;
     this.lines = new CircularList<IBufferLine>(this._getCorrectBufferLength(this._rows));
@@ -68,7 +69,8 @@ export class Buffer implements IBuffer {
     this.scrollBottom = this._rows - 1;
     this.setupTabStops();
     this._memoryCleanupQueue = new IdleTaskQueue(this._logService);
-    this._stringCache = new BufferLineStringCache(() => this._scheduleStringCacheCleanup());
+    this._register(toDisposable(() => this._memoryCleanupQueue.clear()));
+    this._stringCache = this._register(new BufferLineStringCache());
   }
 
   public getNullCell(attr?: IAttributeData): ICellData {
@@ -143,7 +145,7 @@ export class Buffer implements IBuffer {
    * Clears the buffer to it's initial state, discarding all previous data.
    */
   public clear(): void {
-    this._resetStringCache();
+    this._stringCache.clear();
     this.ydisp = 0;
     this.ybase = 0;
     this.y = 0;
@@ -162,7 +164,7 @@ export class Buffer implements IBuffer {
   public resize(newCols: number, newRows: number): void {
     // store reference to null cell with default attrs
     const nullCell = this.getNullCell(DEFAULT_ATTR_DATA);
-    this._resetStringCache();
+    this._stringCache.clear();
 
     // count bufferlines with overly big memory to be cleaned afterwards
     let dirtyMemoryLines = 0;
@@ -310,28 +312,6 @@ export class Buffer implements IBuffer {
     return normalRun;
   }
 
-  private _scheduleStringCacheCleanup(): void {
-    if (this._stringCacheClearTimeout !== undefined) {
-      clearTimeout(this._stringCacheClearTimeout);
-    }
-    this._stringCacheClearTimeout = setTimeout(() => {
-      this._stringCacheClearTimeout = undefined;
-      this._clearStringCache();
-    }, STRING_CACHE_CLEAR_DELAY_MS);
-  }
-
-  private _resetStringCache(): void {
-    if (this._stringCacheClearTimeout !== undefined) {
-      clearTimeout(this._stringCacheClearTimeout);
-      this._stringCacheClearTimeout = undefined;
-    }
-    this._clearStringCache();
-  }
-
-  private _clearStringCache(): void {
-    this._stringCache.clear();
-  }
-
   private get _isReflowEnabled(): boolean {
     const windowsPty = this._optionsService.rawOptions.windowsPty;
     if (windowsPty && windowsPty.buildNumber) {

src/common/buffer/BufferLineStringCache.ts

@@ -4,15 +4,18 @@
  */
 
 import type { IBufferLineStringCache, IBufferLineStringCacheEntry } from 'common/buffer/BufferLine';
+import { disposableTimeout } from 'common/Async';
+import type { IDisposable } from 'common/Lifecycle';
 
-export class BufferLineStringCache implements IBufferLineStringCache {
+const STRING_CACHE_CLEAR_DELAY_MS = 50;
+
+export class BufferLineStringCache implements IBufferLineStringCache, IDisposable {
   public generation: number = 0;
   public readonly entries: Set<IBufferLineStringCacheEntry> = new Set();
-
-  constructor(private readonly _onCacheUsed: () => void = () => { }) { }
+  private _clearTimeout: IDisposable | undefined;
 
   public touch(): void {
-    this._onCacheUsed();
+    this._scheduleClear();
   }
 
   public allocateEntry(): IBufferLineStringCacheEntry {
@@ -22,11 +25,13 @@ export class BufferLineStringCache implements IBufferLineStringCache {
       generation: this.generation
     };
     this.entries.add(entry);
-    this._onCacheUsed();
+    this._scheduleClear();
     return entry;
   }
 
   public clear(): void {
+    this._clearTimeout?.dispose();
+    this._clearTimeout = undefined;
     this.generation++;
     for (const entry of this.entries) {
       entry.value = undefined;
@@ -34,4 +39,16 @@ export class BufferLineStringCache implements IBufferLineStringCache {
     }
     this.entries.clear();
   }
+
+  public dispose(): void {
+    this.clear();
+  }
+
+  private _scheduleClear(): void {
+    this._clearTimeout?.dispose();
+    this._clearTimeout = disposableTimeout(() => {
+      this._clearTimeout = undefined;
+      this.clear();
+    }, STRING_CACHE_CLEAR_DELAY_MS);
+  }
 }

src/common/buffer/BufferSet.test.ts

@@ -6,7 +6,7 @@
 import { assert } from 'chai';
 import { BufferSet } from 'common/buffer/BufferSet';
 import { Buffer } from 'common/buffer/Buffer';
-import { MockOptionsService, MockBufferService, MockLogService } from 'common/TestUtils.test';
+import { MockOptionsService, MockBufferService, MockLogService, createCellData } from 'common/TestUtils.test';
 
 describe('BufferSet', () => {
   let bufferSet: BufferSet;
@@ -82,4 +82,45 @@ describe('BufferSet', () => {
       assert.equal(bufferSet.alt.markers.length, 0);
     });
   });
+
+  describe('lifecycle', () => {
+    it('should dispose previous buffers on reset', () => {
+      const oldNormal = bufferSet.normal as any;
+      oldNormal.lines.get(0)!.setCell(0, createCellData(0, 'a', 1));
+      oldNormal.translateBufferLineToString(0, false);
+
+      const oldCache = oldNormal._stringCache;
+      assert.equal(oldCache.entries.size, 1);
+      assert.notEqual(oldCache._clearTimeout, undefined);
+
+      bufferSet.reset();
+
+      assert.notEqual(bufferSet.normal, oldNormal);
+      assert.equal(oldCache.entries.size, 0);
+      assert.equal(oldCache._clearTimeout, undefined);
+    });
+
+    it('should dispose both buffers when disposed', () => {
+      const normal = bufferSet.normal as any;
+      normal.lines.get(0)!.setCell(0, createCellData(0, 'a', 1));
+      normal.translateBufferLineToString(0, false);
+
+      bufferSet.activateAltBuffer();
+      const alt = bufferSet.alt as any;
+      alt.lines.get(0)!.setCell(0, createCellData(0, 'b', 1));
+      alt.translateBufferLineToString(0, false);
+
+      const normalCache = normal._stringCache;
+      const altCache = alt._stringCache;
+      assert.notEqual(normalCache._clearTimeout, undefined);
+      assert.notEqual(altCache._clearTimeout, undefined);
+
+      bufferSet.dispose();
+
+      assert.equal(normalCache.entries.size, 0);
+      assert.equal(altCache.entries.size, 0);
+      assert.equal(normalCache._clearTimeout, undefined);
+      assert.equal(altCache._clearTimeout, undefined);
+    });
+  });
 });

src/common/buffer/BufferSet.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { Disposable } from 'common/Lifecycle';
+import { Disposable, MutableDisposable } from 'common/Lifecycle';
 import { IAttributeData } from 'common/Types';
 import { Buffer } from 'common/buffer/Buffer';
 import { IBuffer, IBufferSet } from 'common/buffer/Types';
@@ -18,6 +18,8 @@ export class BufferSet extends Disposable implements IBufferSet {
   private _normal!: Buffer;
   private _alt!: Buffer;
   private _activeBuffer!: Buffer;
+  private readonly _normalBuffer = this._register(new MutableDisposable<Buffer>());
+  private readonly _altBuffer = this._register(new MutableDisposable<Buffer>());
 
   private readonly _onBufferActivate = this._register(new Emitter<{ activeBuffer: IBuffer, inactiveBuffer: IBuffer }>());
   public readonly onBufferActivate = this._onBufferActivate.event;
@@ -38,11 +40,13 @@ export class BufferSet extends Disposable implements IBufferSet {
 
   public reset(): void {
     this._normal = new Buffer(true, this._optionsService, this._bufferService, this._logService);
+    this._normalBuffer.value = this._normal;
     this._normal.fillViewportRows();
 
     // The alt buffer should never have scrollback.
     // See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-The-Alternate-Screen-Buffer
     this._alt = new Buffer(false, this._optionsService, this._bufferService, this._logService);
+    this._altBuffer.value = this._alt;
     this._activeBuffer = this._normal;
     this._onBufferActivate.fire({
       activeBuffer: this._normal,