Make dispose more resilient

PerBothner · Dec 27, 2018 · 51e1f49 · 51e1f49
1 parent adbb929
commit 51e1f49
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 6 deletions.
diff --git a/src/EscapeSequenceParser.test.ts b/src/EscapeSequenceParser.test.ts
@@ -1216,7 +1216,7 @@ describe('EscapeSequenceParser', function (): void {
         parser2.parse('\x1b[0m');
         chai.expect(order).eql([3, 2, 1]);
       });
-      it('Dispose', () => {
+      it('Dispose should work', () => {
         const csiCustom: [string, number[], string][] = [];
         parser2.setCsiHandler('m', (params, collect) => csi.push(['m', params, collect]));
         const customHandler = parser2.addCsiHandler('m', (params, collect) => { csiCustom.push(['m', params, collect]); return true; });
@@ -1225,6 +1225,16 @@ describe('EscapeSequenceParser', function (): void {
         chai.expect(csi).eql([['m', [1, 31], ''], ['m', [0], '']]);
         chai.expect(csiCustom).eql([], 'Should not use custom handler as it was disposed');
       });
+      it('Should not corrupt the parser when dispose is called twice', () => {
+        const csiCustom: [string, number[], string][] = [];
+        parser2.setCsiHandler('m', (params, collect) => csi.push(['m', params, collect]));
+        const customHandler = parser2.addCsiHandler('m', (params, collect) => { csiCustom.push(['m', params, collect]); return true; });
+        customHandler.dispose();
+        customHandler.dispose();
+        parser2.parse(INPUT);
+        chai.expect(csi).eql([['m', [1, 31], ''], ['m', [0], '']]);
+        chai.expect(csiCustom).eql([], 'Should not use custom handler as it was disposed');
+      });
     });
     it('EXECUTE handler', function (): void {
       parser2.setExecuteHandler('\n', function (): void {
@@ -1300,7 +1310,7 @@ describe('EscapeSequenceParser', function (): void {
         parser2.parse('\x1b]1;foo=bar\x1b\\');
         chai.expect(order).eql([3, 2, 1]);
       });
-      it('Dispose', () => {
+      it('Dispose should work', () => {
         const oscCustom: [number, string][] = [];
         parser2.setOscHandler(1, data => osc.push([1, data]));
         const customHandler = parser2.addOscHandler(1, data => { oscCustom.push([1, data]); return true; });
@@ -1309,6 +1319,16 @@ describe('EscapeSequenceParser', function (): void {
         chai.expect(osc).eql([[1, 'foo=bar']]);
         chai.expect(oscCustom).eql([], 'Should not use custom handler as it was disposed');
       });
+      it('Should not corrupt the parser when dispose is called twice', () => {
+        const oscCustom: [number, string][] = [];
+        parser2.setOscHandler(1, data => osc.push([1, data]));
+        const customHandler = parser2.addOscHandler(1, data => { oscCustom.push([1, data]); return true; });
+        customHandler.dispose();
+        customHandler.dispose();
+        parser2.parse(INPUT);
+        chai.expect(osc).eql([[1, 'foo=bar']]);
+        chai.expect(oscCustom).eql([], 'Should not use custom handler as it was disposed');
+      });
     });
     it('DCS handler', function (): void {
       parser2.setDcsHandler('+p', {

diff --git a/src/EscapeSequenceParser.ts b/src/EscapeSequenceParser.ts
@@ -316,9 +316,15 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
     if (this._csiHandlers[index] === undefined) {
       this._csiHandlers[index] = [];
     }
-    this._csiHandlers[index].push(callback);
+    const handlerList = this._csiHandlers[index];
+    handlerList.push(callback);
     return {
-      dispose: () => this._csiHandlers[index].splice(this._csiHandlers[index].indexOf(callback))
+      dispose: () => {
+        const handlerIndex = handlerList.indexOf(callback);
+        if (handlerIndex !== -1) {
+          handlerList.splice(handlerIndex);
+        }
+      }
     };
   }
   setCsiHandler(flag: string, callback: (params: number[], collect: string) => void): void {
@@ -345,9 +351,15 @@ export class EscapeSequenceParser extends Disposable implements IEscapeSequenceP
     if (this._oscHandlers[ident] === undefined) {
       this._oscHandlers[ident] = [];
     }
-    this._oscHandlers[ident].push(callback);
+    const handlerList =  this._oscHandlers[ident];
+    handlerList.push(callback);
     return {
-      dispose: () => this._oscHandlers[ident].splice(this._oscHandlers[ident].indexOf(callback))
+      dispose: () => {
+        const handlerIndex = handlerList.indexOf(callback);
+        if (handlerIndex !== -1) {
+          handlerList.splice(handlerIndex);
+        }
+      }
     };
   }
   setOscHandler(ident: number, callback: (data: string) => void): void {