From fca927bb8775874e31858cfbc9f4720fb5f749c4 Mon Sep 17 00:00:00 2001 From: Christoffer Sandberg Date: Fri, 27 May 2022 11:33:32 +0200 Subject: [PATCH 1/3] Extend listener removal in BusHelper - Adds a clean-up function `removeListeners` that in addition to the already existing PropertiesChanged removal on itself (in `Device.disconnect`) also removes PropertiesChanged event listeners on the _propsProxy object. - Adds use of this clean-up function in `Device.disconnect`. --- src/BusHelper.js | 7 +++++++ src/Device.js | 2 +- test/Device.spec.js | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/BusHelper.js b/src/BusHelper.js index ae87ea6..9a6d78e 100644 --- a/src/BusHelper.js +++ b/src/BusHelper.js @@ -97,6 +97,13 @@ class BusHelper extends EventEmitter { return this._ifaceProxy[methodName](...args) } + removeListeners () { + this.removeAllListeners('PropertiesChanged') + if (this._propsProxy !== null) { + this._propsProxy.removeAllListeners('PropertiesChanged') + } + } + static buildChildren (path, nodes) { if (path === '/') path = '' const children = new Set() diff --git a/src/Device.js b/src/Device.js index 9142620..23154c6 100644 --- a/src/Device.js +++ b/src/Device.js @@ -119,7 +119,7 @@ class Device extends EventEmitter { */ async disconnect () { await this.helper.callMethod('Disconnect') - this.helper.removeAllListeners('PropertiesChanged') // might be improved + this.helper.removeListeners() } /** diff --git a/test/Device.spec.js b/test/Device.spec.js index 0cd342e..0679e03 100644 --- a/test/Device.spec.js +++ b/test/Device.spec.js @@ -13,6 +13,7 @@ jest.doMock('../src/BusHelper', () => { this.waitPropChange = jest.fn() this.children = jest.fn() this.callMethod = jest.fn() + this.removeListeners = jest.fn() } } }) From 1cc34e9022b82095feb3c69f22d7f5ab5435d360 Mon Sep 17 00:00:00 2001 From: Christoffer Sandberg Date: Tue, 8 Nov 2022 17:25:28 +0100 Subject: [PATCH 2/3] Fix re-use after listener removal in BusHelper --- src/BusHelper.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/BusHelper.js b/src/BusHelper.js index 9a6d78e..f29da07 100644 --- a/src/BusHelper.js +++ b/src/BusHelper.js @@ -101,6 +101,7 @@ class BusHelper extends EventEmitter { this.removeAllListeners('PropertiesChanged') if (this._propsProxy !== null) { this._propsProxy.removeAllListeners('PropertiesChanged') + this._ready = false } } From 1855c3b4a59b785a6b1f89e69b0b54a11bca2ef6 Mon Sep 17 00:00:00 2001 From: Christoffer Sandberg Date: Tue, 8 Nov 2022 17:33:28 +0100 Subject: [PATCH 3/3] Add test cases for BusHelper.removeListeners --- test/BusHelper.spec.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/BusHelper.spec.js b/test/BusHelper.spec.js index 8e594f3..fbf0da9 100644 --- a/test/BusHelper.spec.js +++ b/test/BusHelper.spec.js @@ -125,3 +125,31 @@ test('propsEvents', async () => { await helper.set('VirtualProperty', buildTypedValue('string', 'bar')) await expect(res).resolves.toMatchObject({ VirtualProperty: { signature: 's', value: 'bar' } }) }) + +test('removeListeners', async () => { + const helper = new BusHelper(dbus, TEST_NAME, TEST_OBJECT, TEST_IFACE, { useProps: true, usePropsEvents: true }) + + const dummyCb = () => {} + + // Init with listener on helper (directly attached dummyCb) and _propsProxy (through method call triggered _prepare) + helper.on('PropertiesChanged', dummyCb) + await helper.callMethod('Echo', 'ping') + expect(helper.listenerCount('PropertiesChanged')).toBeGreaterThan(0) + expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBeGreaterThan(0) + + // Test remove + helper.removeListeners() + expect(helper.listenerCount('PropertiesChanged')).toBe(0) + expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBe(0) + + // Test reuse after remove (same initialization as before) + helper.on('PropertiesChanged', dummyCb) + await helper.callMethod('Echo', 'ping') + expect(helper.listenerCount('PropertiesChanged')).toBeGreaterThan(0) + expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBeGreaterThan(0) + + // Remove second time + helper.removeListeners() + expect(helper.listenerCount('PropertiesChanged')).toBe(0) + expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBe(0) +})