EventEmitter leaks when SCAN_DUPLICATES = true

[cristian-t]

Ever since I switched to Node.js 0.12 I've been getting EventEmitter memory leak warnings. It seems when SCAN_DUPLICATES is set to true, events will be added for every "discovery" of the same sensor tag. I've posted this issue here since I am using ti sensor tags, but I assume this would happen with any BLE device.

[sandeepmistry]

@cristian-t can you please most a code snippet of what you are doing?

[cristian-t]

Here's link to a gist. Running Node 0.10 or 0.12 makes no difference, the leak is there. Node 0.12, however, prints out a warning when it happens. https://gist.github.com/cristian-t/d84c26f6dfef1a506029

SCAN_DUPLICATES is already enabled there, so all you need to do is run it, and have it discover and connect to a device. Then comment out the SCAN_DUPLICATES lines and try again.

Also, I left the reading of the humidity sensor in the snippet as well. I should probably update the gist to show this as well, I forgot. Sensor read events also leak, it's not limited to the "disconnect" event.

[cristian-t]

I've tracked down the issue and believe I fixed it as well. The issue is in noble-device > Util.inherits > constructor.onDiscover A new constructor is created everytime noble emits a discover event, which happens very often for the same device when SCAN_DUPLICATES is true. The new constructor instance then creates a new NobleDevice instance which attaches a disconnect event to the same peripheral over and over again.

This is the temporary fix I applied:

constructor.deviceList = {};

constructor.onDiscover = function(peripheral) {
  if (constructor.is(peripheral)) {
    var device;

    if( constructor.deviceList.hasOwnProperty( peripheral.id ) )
    {
      device = constructor.deviceList[ peripheral.id ];
    }
    else
    {
      device = new constructor(peripheral);
      constructor.deviceList[ peripheral.id ] = device;
    }

    constructor.emitter.emit('discover', device);
  }
};

I have noble-device force each constructor to maintain a deviceList and avoid creating a new constructor instance if it already exists. deviceList is also re-created every time constructor.startScanning is called.

constructor.startScanning = function() {
  constructor.deviceList = {};
  noble.startScanning(constructor.SCAN_UUIDS, constructor.SCAN_DUPLICATES);
};

[sandeepmistry]

@cristian-t this is why setting SensorTag.CC2540.SCAN_DUPLICATES = true; and SensorTag.CC2650.SCAN_DUPLICATES = true; is not something the user should be doing.

Your patch is something that might be worthwhile putting in noble-device.

Based on your gist, it doesn't look like you actually need to discover with duplicates. I think you just need to call discoverAll again after your device disconnects, which your are doing already. Is this correct?

[cristian-t]

That is just a snippet. What I really do is use duplicate discoveries to track rssi and detect whether a device is turned off during discovery

[sandeepmistry]

@cristian-t good to know, those aspects are outside the scope of this module.

I suggest you stick to using noble directly to track RSSI.

[cristian-t]

Well, yes, that's what I was doing now that I think about it, since I was accessing the same peripheral anyway. So duplicate discovery is not necessary to track RSSI. Timing out devices that were turned off during discovery can't be currently done though.