Memory leak in lwgeom_make_valid

[strk]

The lwgeom_make_valid function leaks memory, it's exposed by the test_lwgeom_make_valid test already:

==13713== 240 (16 direct, 224 indirect) bytes in 1 blocks are definitely lost in loss record 10 of 10
==13713==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==13713==    by 0x4E7E23B: GEOS2LWGEOM (lwgeom_geos.c:154)
==13713==    by 0x4E80C10: lwgeom_make_valid (lwgeom_geos_clean.c:1050)
==13713==    by 0x40904C: test_lwgeom_make_valid (cu_clean.c:73)
==13713==    by 0x50A3C99: ??? (in /usr/lib/libcunit.so.1.0.1)
==13713==    by 0x50A4474: CU_run_test (in /usr/lib/libcunit.so.1.0.1)
==13713==    by 0x40542A: main (cu_tester.c:187)

PostGIS-2.0 is _NOT_ affected

[strk]

My fault, looks like. In 2.1 branch:

72da4f60ee11ec8397e5f0eaab1373aef778f065 is the first bad commit
commit 72da4f60ee11ec8397e5f0eaab1373aef778f065
Author: Sandro Santilli <strk@keybit.net>
Date:   Fri Jul 5 10:45:30 2013 +0000

    Backport ST_MakeValid memory leak fix (#2307)
    
    git-svn-id: http://svn.osgeo.org/postgis/branches/2.1@11637 b70326c6-7e19-0410-871a-916f4a2858ee

[strk]

or maybe that's the first commit that _added_ the test, showing the leak

[strk]

The problem is with collection input when output is not a collection anymore, so basically in lwgeom_as_multi, which does an lwgeom_clone() of the passed geometry thus NOT cloning the pointarray. As a result neither of the two POINTARRAY references are marked as READONLY so you can't free one w/out the other being a double-free.

[strk]

Wrong analisys, the cloned geometry does indeed get a READONLY flag set, which means it won't destroy the pointarray. But we created the source geometry and will be giving that one away...

I guess the cleanest thing to do here is to avoid the clone completely and do the "to-multi" locally

[strk]

Fixed by r13026 in 2.1 branch (2.1.5) and r13025 in trunk (2.2.0)