sandeepmistry / node-bleacon

A Node.js library for creating, discovering, and configuring iBeacons
MIT License
497 stars 88 forks source link

Memoryleak with EstimoteSticker? #55

Open T-X opened 8 years ago

T-X commented 8 years ago

Here is a small, minimal test script which seems to expose a memory leak:

https://gist.github.com/T-X/99b18a3bb1667568245f823dcd19149f

I am running it via:

$ NOBLE_REPORT_ALL_HCI_EVENTS=1 node --expose-gc ./test-leak.js

Resulting in heapdump files like these:

http://metameute.de/~tux/bugreport/node-bleacon/heapdump-2016-06-25/

The first one is from after one minute of runtime, the second one at 30min. runtime, the last one at 60min.

Looking at them via the chromium profiler, it seems that after one hour there are more than 110'000 objects which look like:

{ advertisement, __proto__, address, addressType }

The heapUsed console log output grows constantly, too (one line per minute):

xxx heapUsed:5583776
xxx heapUsed:6281624
xxx heapUsed:6588352
xxx heapUsed:6953632
xxx heapUsed:7267560
xxx heapUsed:7628656
xxx heapUsed:8016104
xxx heapUsed:10719920
xxx heapUsed:11081072
xxx heapUsed:11426216
xxx heapUsed:11775760
xxx heapUsed:12042152
xxx heapUsed:12422480
...

The phyisical setup contains 21 Estimote stickers in range of the bluetooth dongle.

T-X commented 8 years ago

PS: I'm not really experienced with memory leak debugging in node/javascript. But the Retainers tree looks similar to the memory leak debugging blog article here:

http://blog.yld.io/2015/08/10/debugging-memory-leaks-in-node-js-a-walkthrough/

This one is talking about a signal handler in its example. And in the linked heapdump output for the estimote sticker, there is a process thingy at distance one in the retainers tree, too. A handler for the exit signal, registered in native_bind() in NobleBindings might be keeping a reference for too long?

sandeepmistry commented 8 years ago

Hi @T-X,

Looking at them via the chromium profiler, it seems that after one hour there are more than 110'000 objects which look like: { advertisement, proto, address, addressType }

Do you think this is a noble Peripheral object?

https://github.com/sandeepmistry/noble/blob/master/lib/peripheral.js#L7

I don't have any functioning Estimote stickers (batteries died). However, I'm wondering if they rotate BT addresses. noble book keeps all discovered peripheral by BT address ...

You could try to track peripheral.address in estimote-sticker.js to see what's going on ...

T-X commented 8 years ago

Hi @sandeepmistry,

Thanks for your reply (and this awesome nodejs library :) !).

Here is an example output of "$ hcitool lescan" with just one estimote sticker in range and some annotations:

# $ stdbuf -o L hcitool lescan | stdbuf -o L ts -i "%.s" | stdbuf -o L tee /tmp/hcitool-lescan.log
0.000018 LE Scan ...
1.749553 E3:30:27:07:19:7D (unknown)
2.601901 E3:48:17:58:BA:7D (unknown)
2.603893 E3:3A:91:03:E1:7D (unknown)
2.595847 E3:30:F4:F2:ED:7D (unknown)
2.605909 E3:52:12:9F:DC:7D (unknown)
2.600916 E3:45:80:E4:4C:7D (unknown)
2.599895 E3:31:96:78:50:7D (unknown)
# Movement started
0.997869 E3:00:02:4F:71:7D (unknown)
0.110997 E3:AA:BF:B0:F4:7D (unknown)
0.001009 E3:AA:BF:B0:F4:7D (unknown)
0.111066 E3:7F:79:E9:55:7D (unknown)
0.117913 E3:AA:BF:B0:F4:7D (unknown)
0.000958 E3:AA:BF:B0:F4:7D (unknown)
0.106036 E3:3B:19:E7:89:7D (unknown)
1.310942 E3:AA:BF:B0:F4:7D (unknown)
0.000956 E3:AA:BF:B0:F4:7D (unknown)
1.302981 E3:23:6F:26:43:7D (unknown)
0.264061 E3:46:D1:3C:68:7D (unknown)
0.111007 E3:06:BB:21:5E:7D (unknown)
0.113936 E3:0F:59:81:15:7D (unknown)
0.116980 E3:AA:BF:B0:F4:7D (unknown)
0.000955 E3:AA:BF:B0:F4:7D (unknown)
0.103027 E3:AA:BF:B0:F4:7D (unknown)
0.000958 E3:AA:BF:B0:F4:7D (unknown)
# Movement stopped here
1.311053 E3:63:3E:F7:35:7D (unknown)
1.302874 E3:3F:03:B7:AE:7D (unknown)
2.610949 E3:4B:61:88:5B:7D (unknown)
1.298863 E3:32:7D:5C:D6:7D (unknown)
1.313957 E3:73:F0:66:1E:7D (unknown)
1.294943 E3:AA:BF:B0:F4:7D (unknown)
0.001017 E3:AA:BF:B0:F4:7D (unknown)
1.305904 E3:59:D6:75:C4:7D (unknown)
1.305905 E3:4C:37:ED:FA:7D (unknown)
1.308991 E3:50:9B:B9:95:7D (unknown)
1.311915 E3:36:E2:28:77:7D (unknown)
# Beaconing interval back to normal here
2.596941 E3:57:70:D9:CF:7D (unknown)
2.603853 E3:05:4B:4E:71:7D (unknown)
2.603847 E3:1B:EF:C1:90:7D (unknown)
2.603922 E3:6D:D5:28:B3:7D (unknown)
2.594891 E3:10:E2:6C:A7:7D (unknown)

So indeed looks like regular beacons have a random MAC address, except the first and last byte (while the beacons during movement have a fixed, unique per sticker one).

Btw., interestingly I had to add "NOBLE_REPORT_ALL_HCI_EVENTS=1" to actually get those beacons with a random MAC into the discover event of EstimoteSticker. And the only way to get reliable, periodic discover events for non-moving stickers at all.

Also those heaps of objects in the format "{ advertisement, proto, address, addressType }" in the heapdump file have an "addressType = random".

So looks like your initial conjecture is on the right track? :)

Out of curiousity, why are they kept and not thrown away after all discover handlers finished, for instance? To be able to log and track state changes of a beacon?

Sorry, not a javascript expert at all, but yes, peripheral.js#L7 looks similar indeed.

Just for completeness, advertisement and proto seem to be pointers to other, leaking objects, too: "{ advertisement, proto, address, addressType }" -> 80 bytes alloc. size "{ manufacturerData, proto, serviceData, serviceUuids } (= advertisement)" -> 64 bytes "{ proto } (= proto)" -> 56 bytes

While address and addressType are just strings.

(as you can see and check in the included heapdump files in my initial post).

sandeepmistry commented 8 years ago

Btw., interestingly I had to add "NOBLE_REPORT_ALL_HCI_EVENTS=1" to actually get those beacons with a random MAC into the discover event of EstimoteSticker. And the only way to get reliable, periodic discover events for non-moving stickers at all.

Yes, this is because noble currently excepts peripherals to have a scan response, however the Estimote stickers do not.

Out of curiousity, why are they kept and not thrown away after all discover handlers finished, for instance? To be able to log and track state changes of a beacon?

Might not be needed any more, but it was to have a reference to the peripheral for when peripheral.connect() might be called in the future. Original the address and address type were not stored in the peripheral object. Something to think about removing ...