Fix memory leaks (thanks @amlweems)

[juanjux]

(Branched from amlweems:fix-memory-leak)

• Apply the memory leaks detected and fixed by @amlweems with an aditional one for the String helper functions that allows the fix for ItemAt to work.

• Test (also from @amlweems branch) to check that memory doesn't increase after 100 filter() calls.

Signed-off-by: Juanjo Alvarez juanjo@sourced.tech

[juanjux]

cc @amlweems

[vmarkovtsev]

LGTM, but I recommend running the test suite under valgrind and CPython configured with --without-pymalloc --with-pydebug --with-valgrind just to make sure we have not missed anything.

[amlweems]

Nice! Testing with pympler again, this fixes the major memory leak. And the objects that are still around don't scale with the number of iterations I do against filter().

# 5000 iterations of filter()
        types |   # objects |   total size
============= | =========== | ============
  <class list |        5056 |    555.20 KB
   <class str |        5045 |    438.51 KB
   <class int |        1003 |     43.10 KB
...

# 10000 iterations of filter()
        types |   # objects |   total size
============= | =========== | ============
  <class list |        5056 |    555.20 KB
   <class str |        5045 |    438.51 KB
   <class int |        1003 |     43.10 KB
...

I tried running with cpython and valgrind. I get a lot of "possibly lost", but no worrying "definitely lost". I'm not super confident in these results, but I am very happy with the object counts above. Full output attached.

# cpython build conf
./configure --without-pymalloc --with-pydebug --with-valgrind --prefix=`pwd`/build

# valgrind command
valgrind --tool=memcheck --suppressions=/tmp/cpython/Misc/valgrind-python.supp --log-file=leaks.txt --leak-check=full python -m unittest bblfsh.test.BblfshTests.testManyFilters

==21838== LEAK SUMMARY:
==21838==    definitely lost: 40 bytes in 1 blocks
==21838==    indirectly lost: 0 bytes in 0 blocks
==21838==      possibly lost: 6,314,378 bytes in 37,480 blocks
==21838==    still reachable: 193,285 bytes in 310 blocks
==21838==         suppressed: 0 bytes in 0 blocks

[juanjux]

@vmarkovtsev that would be certainly great, I used a simpler tool that doesn't required Python to recompile called memleax, I'm pretty sure current leaks are fixed since I increased locally the iterations in the memory test to 100.000 and the memory didn't increase, but I'll give a run with Valgrind when I've a little time.

On the other side, Travis is failing with a core dump while its working perfectly on my machine :/

[vmarkovtsev]

@amlweems This leak summary gives a solid confidence that all the leaks are fixed, awesome!

[juanjux]

Found the reason why it works on my machine(TM) and not in Travis... when testing for leaks, I set the environment variable "PYTHONMALLOC" to "malloc" since that allows the memleak command to find the leaks. With that environment variable set, it works, without it, it segfaults. Looks like I'm for another debugging session 👎

[juanjux]

Looks like the problem comes from mixing malloc with pymalloc in the same extension module. This is not a problem for shared libraries, but since libuast is compiled with the extension module it causes problems. The solutions I'm evaluating are requiring and using libuast.so or adding a function to libuast to pass the alloc/free functions to use.

[juanjux]

Ok, the pointer-between-different-mallocs problem is hellish and using the libuast.so crashed just the same, so I did a workaround that while not perfect will avoid the leak: I store the problematic PyObjects in a PyList and free (DECREF) it at the end of Python's filter() wrapper when libuast is not touching them anymore for sure.

[vmarkovtsev]

Amazing detective work @juanjux !