MaxListenersExceededWarning: Possible EventEmitter memory leak detected when calling device::gatt()

[naugehyde]

Running the code below with appropriate command line args (mac_address and timeout. eg node test.js 'FF:FF:FF:FF:FF:FF' 15000) causes, after 10 trials in my case, the following warning to appear on the console repeatedly, one for each of the gatt server's primary service's characteristics:

(node:230077) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 {"path":"/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/service000e/char00XX","interface":"org.freedesktop.DBus.Properties","member":"PropertiesChanged"} listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

const {createBluetooth} = require('node-ble')
const {bluetooth, destroy} = createBluetooth()
const { argv } = require('node:process');


async function main(){

  const adapter = await bluetooth.defaultAdapter()
  const device = await adapter.waitDevice(argv[2])
  
  try{await adapter.startDiscovery()} catch{}
  var trials=0

  async function search(){
      await device.connect()
      console.log(`GATT trial #${++trials}...`)
      const gatt = await device.gatt()
      await device.disconnect()
 }

setInterval(() => {
  search()
},argv[3]);
search()
}
main()

[Regina-v]

Running into the same issue and applying #79 just breaks everything.

I have a simple test script that continuously scans for devices and if it finds one my devices, it connects, prints the services, and disconnects.

await this.device.connect();
const gattServer = await this.device.gatt();
const services = await gattServer.services();
this.log(`├---gatt server services: ${services}`, 'green');
await this.device.disconnect();

After some time (connecting to the same device for the 11th time?) it starts to throw the max listeners exceeded warning.

What I think happens:

• device.gatt() initiates a new instance of GattServer and calls gattServer.init()
• gattServer.init() initiates a new instance of GattService for each service and calls service.init()
• service.init() initiates a new instance of GattCharacteristic for each characteristic and calls characteristic.getUUID()
• characteristic.getUUID() calls this.helper.prop('UUID')
• this.helper.prop('UUID') calls BusHelper._prepare()
• This creates an event listener this._propsProxy.on('PropertiesChanged') in an if condition which is only true for the GattCharacteristic class.
• ⚡️ Every time I connect to a device, one event listener for every characteristic of every servcie is added which is never removed.
• device.disconnect() has a this.helper.removeListeners() but this only removes the listeners which were created in the BusHelper instance of the Device instance. It's not removing the event listeners of all BusHelper instances of all GattCharacteristic.

I don't yet have a very good understanding of this library. Currently I could think of (but not being able to implement) two solutions:

1. Only add the event listener for a GattCharacteristic within startNotifications. I don't exactly know what this._propsProxy.on('PropertiesChanged') is doing. Is it actually needed?
2. Extend device.disconnect() to not only clean up event listeners of device but iterate over all services and characteristics and clean up their event listeners as well.

[naugehyde]

That's the problem in a nutshell. Good explainer. Thank you!

NB: In a class that required frequent reconnects to a bluetooth device, I stored the result of Device::gatt() in an instance variable and before reconnecting I made sure to execute the following:

if (this.gattServer) {
      (await this.gattServer.services()).forEach(async (uuid) => {
        await this.gattServer.getPrimaryService(uuid).then(async (service) => {
          (await service.characteristics()).forEach(async (uuid) => {
            (await service.getCharacteristic(uuid)).helper.removeListeners();
          });
          service.helper.removeListeners();
        });
        this.gattServer.helper.removeListeners();
      });
    }

Also, I noticed that even with this code in place, after a large number of reconnects, I'd get a cascade of DBus errors that looked like:

Connection ":1.xxxxxx" is not allowed to add more match rules (increase limits in configuration file if required; max_match_rules_per_connection=1024

But couldn't trace it to its source.

[naugehyde]

I wonder if the following change to BusHelper::children() would make a difference:

async children () {
    this._ready = false // WORKAROUND: it forces to construct a new ProxyObject
    if (this._propsProxy) this._propsProxy.removeAllListeners() // *NEW* clear the listeners for the _propsProxy before rebuilding
    await this._prepare()
    return BusHelper.buildChildren(this.object, this._objectProxy.nodes)

 }

[Regina-v]

I had written some workaround patch in my project to remove the event listeners and it works but your code looks much cleaner 👍

For the match rules issue: this is a known issue in the dbus-next dependency, which is not maintained anymore. JellyBrick has forked it and fixed this https://github.com/JellyBrick/node-dbus-next/pull/4 and some more issues. Overriding the dbus-next dependency

"overrides": { "node-ble>dbus-next": "npm:@jellybrick/dbus-next@^0.10.2" }

fixed the match rule issue for me.

[naugehyde]

Nice! Thank you for that!