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

Memory leak - #29

Open
denverquane wants to merge 4 commits into
paulsmith:masterfrom
denverquane:memoryLeak
Open

Memory leak#29
denverquane wants to merge 4 commits into
paulsmith:masterfrom
denverquane:memoryLeak

Conversation

@denverquane

Copy link
Copy Markdown

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