Extend listener removal in BusHelper

[tuxedoxt]

Hello,

if the intended use of Device.disconnect also is to clean up the remaining event listeners this PR extends this to include the leftover BusHelper._propsProxy event listeners adding a new function BusHelper.removeListeners for this purpose.

This seems to do it for my current issue in #35.

[tuxedoxt]

Bump :)

Any feedback?

[chrvadala]

This approach looks better, but I have to try it. I will next weekend, promised.
In the mean time, can you add some unit test?

[ChocolateLoverRaj]

This seems to do it for my current issue in #35.

@tuxedoxt How did you get the error to stop? I tried using your fork and calling await device.disconnect(), but I still got error.

[mxc42]

Related issue, after calling Adapter.devices in a loop once every 5 seconds I start to get (node:4278) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 {"path":"/org/bluez/hci0/dev_6C_EC_26_F4_E4_5C","interface":"org.freedesktop.DBus.Properties","member":"PropertiesChanged"} listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

This is the code in question, called from a setInterval loop every 5 seconds.

    const devices = await adapter.devices();
    for (let i = 0; i < devices.length; i++) {
        try {

            const device = await adapter.getDevice(devices[i]);
            const name = await device.getName();
            const rssi = await device.getRSSI();
            await device.disconnect();
            ...

I've tried the changes in this PR but they didn't affect the error.

[tuxedoxt]

@tuxedoxt How did you get the error to stop? I tried using your fork and calling await device.disconnect(), but I still got error.

I ended up using this change combined with dbusjs/node-dbus-next#110

Since none got merged still using the forked versions explicitly binding the github urls in package.json where needed.

Example, temporary forked node-ble which has this change and a reference to forked dbus-next change:

"node-ble": "git+https://github.com/tuxedoxt/node-ble.git#match-and-event-leak-fixes",

[chrvadala]

I confirm that exists a memory leak, but the provided fix may introduce a different bug.

[chrvadala]

@tuxedoxt I understand that you want to delete this listener because it generate a memory leak, but I think that if you do this in this way, you creates a different bug.
What if the user connects again this device? No code line will subscribe again on 'PropertiesChanged' events, so the library will not work correctly.

I think that this bug requires a different fix.

What do you think?

[tuxedoxt]

Thanks for the feedback. I see what you mean.

As I see it, additionally setting

this._ready = false

should make _prepare reattach listener. However also does other things, like assigns completely new _propsProxy and _ifaceProxy objects. Would more clean-up be needed there or is this feasible?

[coveralls]

Coverage: 95.783% (-0.05%) from 95.833% when pulling 1855c3b on tuxedoxt:device-listener-cleanup into eb20e9c on chrvadala:main.

[derwehr]

Any chance this will get merged soon?

[tuxedoxt]

Another bump :)

@chrvadala

[Leafgard]

Is There Anybody Out There?

@chrvadala

[chrvadala]

Released with v1.10