josephg / node-foundationdb

Modern Node.js FoundationDB bindings
Other
116 stars 17 forks source link

Valgrind / ??? memory leak tests #10

Closed josephg closed 6 years ago

josephg commented 6 years ago

We should run valgrind against the native code. I suspect there's a few small memory leaks - its hard to tell without some good tooling to instrument everything.

Run valgrind. Fix errors. Report back.

josephg commented 6 years ago
$ valgrind node dist/test.js
==11928== Memcheck, a memory error detector
==11928== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==11928== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==11928== Command: node dist/test.js
==11928== 
==11928== Warning: set address range perms: large range [0x3b767bf53000, 0x3b769bf54000) (noaccess)
Setting val to 0
....................................................................................................
Value is now 100 after 837 commit attempts
==11928== Warning: set address range perms: large range [0x3b767bf53000, 0x3b769bf53000) (noaccess)
==11928== 
==11928== HEAP SUMMARY:
==11928==     in use at exit: 373,107 bytes in 2,063 blocks
==11928==   total heap usage: 156,184 allocs, 154,121 frees, 52,329,102 bytes allocated
==11928== 
==11928== LEAK SUMMARY:
==11928==    definitely lost: 3,488 bytes in 102 blocks
==11928==    indirectly lost: 2,280 bytes in 95 blocks
==11928==      possibly lost: 76,215 bytes in 12 blocks
==11928==    still reachable: 291,124 bytes in 1,854 blocks
==11928==                       of which reachable via heuristic:
==11928==                         stdstring          : 11,462 bytes in 198 blocks
==11928==         suppressed: 0 bytes in 0 blocks
==11928== Rerun with --leak-check=full to see details of leaked memory
==11928== 
==11928== For counts of detected and suppressed errors, rerun with: -v
==11928== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

A lot of it is weird things that probably aren't my fault. But there's definitely a few objects which aren't being cleaned up properly.

josephg commented 6 years ago

It actually looks ok. I ran tests like this:

(async () => {
  runTests()
  gc()
})()

and node --expose-gc test.js. All the transactions are being cleaned up correctly. The test is leaking a bunch of small objects that aren't in any of my stacks. I'm going to call this a wash for now.

We should re-run leak checks when the fdb tester is running too.