Skip to content

Commit

Permalink
Merge branch 'device-listener-cleanup-v2' into match-and-event-leak-f…
Browse files Browse the repository at this point in the history
…ixes-v2
  • Loading branch information
tuxedoxt committed Jan 15, 2024
2 parents 680a022 + 89b0bb8 commit 4b7cb1c
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/Adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ class Adapter {

/**
* Init a device instance and returns it.
* Caller is responsible of calling cleanup() on the
* returned Device before discarding it to free resources.
* @param {string} uuid - Device Name.
* @async
* @returns {Device}
Expand Down
8 changes: 8 additions & 0 deletions src/BusHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ class BusHelper extends EventEmitter {
return this._ifaceProxy[methodName](...args)
}

removeListeners () {
this.removeAllListeners('PropertiesChanged')
if (this._propsProxy !== null) {
this._propsProxy.removeAllListeners('PropertiesChanged')
this._ready = false
}
}

static buildChildren (path, nodes) {
if (path === '/') path = ''
const children = new Set()
Expand Down
9 changes: 8 additions & 1 deletion src/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class Device extends EventEmitter {
*/
async disconnect () {
await this.helper.callMethod('Disconnect')
this.helper.removeAllListeners('PropertiesChanged') // might be improved
await this.cleanup()
}

/**
Expand All @@ -142,6 +142,13 @@ class Device extends EventEmitter {

return `${name} [${address}]`
}

/**
* Cleanup when discarding device object
*/
async cleanup () {
this.helper.removeListeners()
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ declare namespace NodeBle {
disconnect(): Promise<void>;
gatt(): Promise<GattServer>;
toString(): Promise<string>;
cleanup(): Promise<void>;

on(event: 'connect', listener: (state: ConnectionState) => void): this;
on(event: 'disconnect', listener: (state: ConnectionState) => void): this;
Expand Down
28 changes: 28 additions & 0 deletions test/BusHelper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
1 change: 1 addition & 0 deletions test/Device.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jest.doMock('../src/BusHelper', () => {
this.waitPropChange = jest.fn()
this.children = jest.fn()
this.callMethod = jest.fn()
this.removeListeners = jest.fn()
}
}
})
Expand Down

0 comments on commit 4b7cb1c

Please sign in to comment.