Merge pull request #4671 from Tyriar/4652

Fix SortedList.values iteration and general dec lifecycle fixes

addons/xterm-addon-search/src/SearchAddon.ts

@@ -68,8 +68,6 @@ export class SearchAddon extends Disposable implements ITerminalAddon {
   private _highlightDecorations: IHighlight[] = [];
   private _selectedDecoration: IHighlight | undefined;
   private _highlightLimit: number;
-  private _onDataDisposable: IDisposable | undefined;
-  private _onResizeDisposable: IDisposable | undefined;
   private _lastSearchOptions: ISearchOptions | undefined;
   private _highlightTimeout: number | undefined;
   /**
@@ -93,13 +91,9 @@ export class SearchAddon extends Disposable implements ITerminalAddon {
 
   public activate(terminal: Terminal): void {
     this._terminal = terminal;
-    this._onDataDisposable = this.register(this._terminal.onWriteParsed(() => this._updateMatches()));
-    this._onResizeDisposable = this.register(this._terminal.onResize(() => this._updateMatches()));
-    this.register(toDisposable(() => {
-      this.clearDecorations();
-      this._onDataDisposable?.dispose();
-      this._onResizeDisposable?.dispose();
-    }));
+    this.register(this._terminal.onWriteParsed(() => this._updateMatches()));
+    this.register(this._terminal.onResize(() => this._updateMatches()));
+    this.register(toDisposable(() => this.clearDecorations()));
   }
 
   private _updateMatches(): void {

src/browser/decorations/BufferDecorationRenderer.ts

@@ -103,6 +103,10 @@ export class BufferDecorationRenderer extends Disposable {
         decoration.element = element;
         this._decorationElements.set(decoration, element);
         this._container.appendChild(element);
+        decoration.onDispose(() => {
+          this._decorationElements.delete(decoration);
+          element!.remove();
+        });
       }
       element.style.top = `${line * this._renderService.dimensions.css.cell.height}px`;
       element.style.display = this._altBufferIsActive ? 'none' : 'block';

src/common/SortedList.test.ts

@@ -104,4 +104,18 @@ describe('SortedList', () => {
       { key: 10 }
     ]);
   });
+  describe('values', () => {
+    it('should iterate correctly when list items change during iteration', () => {
+      list.insert(1);
+      list.insert(2);
+      list.insert(3);
+      list.insert(4);
+      const visited: number[] = [];
+      for (const item of list.values()) {
+        visited.push(item);
+        list.delete(item);
+      }
+      deepStrictEqual(visited, [1, 2, 3, 4]);
+    });
+  });
 });

src/common/SortedList.ts

@@ -89,7 +89,8 @@ export class SortedList<T> {
   }
 
   public values(): IterableIterator<T> {
-    return this._array.values();
+    // Duplicate the array to avoid issues when _array changes while iterating
+    return [...this._array].values();
   }
 
   private _search(key: number): number {

src/common/buffer/Marker.ts

@@ -11,9 +11,9 @@ export class Marker implements IMarker {
   private static _nextId = 1;
 
   public isDisposed: boolean = false;
-  private _disposables: IDisposable[] = [];
+  private readonly _disposables: IDisposable[] = [];
 
-  private _id: number = Marker._nextId++;
+  private readonly _id: number = Marker._nextId++;
   public get id(): number { return this._id; }
 
   private readonly _onDispose = this.register(new EventEmitter<void>());

src/common/services/DecorationService.ts

@@ -35,12 +35,7 @@ export class DecorationService extends Disposable implements IDecorationService
   constructor() {
     super();
 
-    this.register(toDisposable(() => {
-      for (const d of this._decorations.values()) {
-        this._onDecorationRemoved.fire(d);
-      }
-      this.reset();
-    }));
+    this.register(toDisposable(() => this.reset()));
   }
 
   public registerDecoration(options: IDecorationOptions): IDecoration | undefined {
@@ -92,13 +87,6 @@ export class DecorationService extends Disposable implements IDecorationService
       }
     });
   }
-
-  public dispose(): void {
-    for (const d of this._decorations.values()) {
-      this._onDecorationRemoved.fire(d);
-    }
-    this.reset();
-  }
 }
 
 class Decoration extends Disposable implements IInternalDecoration {