Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Memory leak #29

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Memory leak #29

wants to merge 4 commits into from

Conversation

denverquane
Copy link

The wkt encoding function leaks memory, as it does not free the C String allocated when encoding.

You can verify this with a simple example that encodes a polygon to a wkt string, and analyze the executable with Valgrind:

valgrind --leak-check=full --show-leak-kinds=all ./main
...
==26520== LEAK SUMMARY:
==26520==    definitely lost: 41,100 bytes in 100 blocks
==26520==    indirectly lost: 0 bytes in 0 blocks
==26520==      possibly lost: 3,552 bytes in 6 blocks
==26520==    still reachable: 250,616 bytes in 2,488 blocks
==26520==         suppressed: 0 bytes in 0 blocks

And here is an example of one of the lost memory records (evidently a trace to the wkt encoding):

==26520== 411 bytes in 1 blocks are definitely lost in loss record 228 of 363
==26520==    at 0x4C29F73: malloc (vg_replace_malloc.c:309)
==26520==    by 0x4E4BF34: (anonymous namespace)::gstrdup_s(char const*, unsigned long) (geos_ts_c.cpp:311)
==26520==    by 0x4E562CE: gstrdup (geos_ts_c.cpp:330)
==26520==    by 0x4E562CE: operator() (geos_ts_c.cpp:2516)
==26520==    by 0x4E562CE: execute<GEOSWKTWriter_write_r(GEOSContextHandle_t, geos::io::WKTWriter*, const geos::geom::Geometry*)::__lambda140, nullptr> (geos_ts_c.cpp:379)
==26520==    by 0x4E562CE: GEOSWKTWriter_write_r (geos_ts_c.cpp:2518)
==26520==    by 0x4CA94E: _cgo_d650cf0434ad_Cfunc_GEOSWKTWriter_write_r (cgo-gcc-prolog:3024)
==26520==    by 0x45B42F: runtime.asmcgocall (/usr/local/go/src/runtime/asm_amd64.s:655)
==26520==    by 0xC00018A137: ???
==26520==    by 0x51541F: ??? (in /home/***)

Compare this same test with just the fix provided in this PR:

valgrind --leak-check=full --show-leak-kinds=all ./main
...
==26894== LEAK SUMMARY:
==26894==    definitely lost: 0 bytes in 0 blocks
==26894==    indirectly lost: 0 bytes in 0 blocks
==26894==      possibly lost: 3,552 bytes in 6 blocks
==26894==    still reachable: 232,448 bytes in 2,302 blocks
==26894==         suppressed: 0 bytes in 0 blocks

Valgrind shouldn't typically be used for analyzing executables built with Go because of the GC, but in this context we can see that the version that doesn't free the C String certainly leaks memory (as it did with our deployment, and we traced it back to this function).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant