Fix a couple of cpu profile memory leaks

[dylanahsmith]

Problem

I tried using valgrind to check for leaks in v8go and found some in the cpu profiling code that can be reproduced using go test -c && valgrind --leak-check=full --show-leak-kinds=definite --track-origins=yes ./v8go.test -test.run=TestCPUProfile (run on linux with valgrind already installed)

Here are the unique leaks that valgrind detected

==321754== 8 bytes in 1 blocks are definitely lost in loss record 569 of 894
==321754==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==321754==    by 0xA8169D: NewCPUProfileNode (v8go.cc:239)
==321754==    by 0xA89EC4: CPUProfilerStopProfiling (v8go.cc:275)
==321754==    by 0xA7F55E: _cgo_eb9c209376d5_Cfunc_CPUProfilerStopProfiling (cgo-gcc-prolog:78)
==321754==    by 0x8705AF: runtime.asmcgocall.abi0 (asm_amd64.s:765)
==321754==    by 0xC000027FFF: ???
==321754==    by 0x2DFECDD7: ???
==321754==    by 0x849117: runtime.exitsyscallfast.func1 (proc.go:4017)
==321754==    by 0x86E708: runtime.systemstack.abi0 (asm_amd64.s:383)
==321754==
==321754== 8 bytes in 2 blocks are definitely lost in loss record 570 of 894
==321754==    at 0x483C583: operator new[](unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==321754==    by 0xA8169D: NewCPUProfileNode (v8go.cc:239)
==321754==    by 0xA816BB: NewCPUProfileNode (v8go.cc:241)
==321754==    by 0xA89EC4: CPUProfilerStopProfiling (v8go.cc:275)
==321754==    by 0xA7F55E: _cgo_eb9c209376d5_Cfunc_CPUProfilerStopProfiling (cgo-gcc-prolog:78)
==321754==    by 0x8705AF: runtime.asmcgocall.abi0 (asm_amd64.s:765)
==321754==    by 0x1A0C7BF: ???
==321754==    by 0x1: ???
==321754==    by 0xC000044DCF: ???

Solution

I added the missing delete for the C++ allocated memory and free for the malloc allocated memory.

[codecov]

Codecov Report

Merging #218 (be55804) into master (cc3b2e8) will not change coverage.
The diff coverage is n/a.

❗ Current head be55804 differs from pull request most recent head 9af31fd. Consider uploading reports for the commit 9af31fd to get more accurate results

@@           Coverage Diff           @@
##           master     #218   +/-   ##
=======================================
  Coverage   95.59%   95.59%           
=======================================
  Files          16       16           
  Lines         545      545           
=======================================
  Hits          521      521           
  Misses         15       15           
  Partials        9        9           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc3b2e8...9af31fd. Read the comment docs.

[dylanahsmith]

Changing to

Suggested change

	delete node->children;
	delete[] node->children;

to avoid a Mismatched free() / delete / delete [] error

[dylanahsmith]

I reran valgrind with go test -c && valgrind --leak-check=full --show-leak-kinds=definite --track-origins=yes ./v8go.test -test.run=TestCPUProfile and it didn't find any leaks.

[genevieve]

I wonder if it would make sense to document this in a DEVELOPMENT type doc and/or run valgrind in our ci flow against the whole test suite. Not everyone might have it installed on their machines and it would be a helpful ci check on PRs