[postgis-devel] memory management questions #1

[Nicklas Avén]

Thanks a lot Paul. I'm learning :-)


Your point about the point lists not copied, I think I should have known
by now, but I have not seen the problem this obvious before.

But what happens if I try to free a LWGEOM with point lists pointing
right into the database memory with lwgeom_free?

That is what you are doing in st_collectionextract on line 4081 in
lwgeom__functions__basic.c, isn't it? 

The only leak I have found so far in the distance code is that I create
bboxes that I don't free. I thought I could fix taht by just fixing the
mentioned bug in lwpoint_free and then free the deserialized LWGEOM in
lwgeom__functions__basic, but that is maybe not such a good idea.

I guess it is better to wait a little to see what comes from your work
now and to see if all the distance code will be changed before I put too
much work in it. 

I thought I should fix 2D and 3D at the same time, that's why I started
now. 

Thanks
Nicklas




On Sun, 2010-10-24 at 15:17 -0700, Paul Ramsey wrote:
> On Sun, Oct 24, 2010 at 2:37 PM, Nicklas Avén
> <nicklas.aven at jordogskog.no> wrote:
> > I think lwgeom_free and lwgeom_release seems to do much the same thing.
> > Why do we have both? When to use which one?
> 
> There are serious inconsistencies in our free/release API which you
> are seeing. Fundamentally there isn't much difference anymore. An
> important thing to understand, though, is that an LWGEOM deserialized
> from a database PGLWGEOM will have point arrays with
> serialzed_pointlists that back right onto the database memory. Which
> means you cannot free those pointlists.
> 
> Contrariwise, a geometry you construct yourself from WKT or something,
> will have completely free floating serialized_pointlists, which you
> should deallocate before leaving.
> 
> My hope for 2.0 is to include in the geometry/pointarray 'flags' a
> flag that indicates whether the underlying serialized_pointlist arrays
> are managed by teh database or by the lwgeom, so that lwgeom_free can
> magically "do the right thing".
> 
> As it stands now, it's a mess and there is no consistency, and some
> code has manually implemented things like freeing right to the bottom
> rather than using the API.
> 
> >> I found that the bbox was not released when calling lwgeom_free with points.
> > For other types there is a check for bbox and they are freed but not for
> > points in lwpoint_free.
> >
> > Is that for some reason or is it a bug?
> 
> Probably a bug, based on the expectation that most point objects won't
> have bounding boxes.
> 
> > Then I have a third question that maybe wrong time to ask since Paul is
> > working on the parser. But I get memory leaks when using lwgeom_from_ewkt
> > looking like this:
> >
> > ==32144== 37 bytes in 1 blocks are definitely lost in loss record 2 of 3
> > ==32144==    at 0x4C274A8: malloc (vg_replace_malloc.c:236)
> > ==32144==    by 0x42BB7F: make_serialized_lwgeom (lwgparse.c:1387)
> > ==32144==    by 0x42CA2A: parse_it (lwgparse.c:1730)
> > ==32144==    by 0x417253: serialized_lwgeom_from_ewkt (lwgeom.c:758)
> > ==32144==    by 0x417DB3: lwgeom_from_ewkt (lwgeom.c:736)
> > ==32144==    by 0x40A02F: test_mindistance2d_tolerance (cu_measures.c:35)
> > ==32144==    by 0x50B32A9: ??? (in /usr/lib/libcunit.so.1.0.1)
> > ==32144==    by 0x50B3ACE: CU_run_test (in /usr/lib/libcunit.so.1.0.1)
> > ==32144==    by 0x411B6B: main (cu_tester.c:138)
> 
> You're in the old parser (parse_it is called) so it's Not My Problem.
> Which isn't to say that my parser won't leak, I am, frankly, being a
> little lazy knowing that palloc/pfree are going to be behind me
> cleaning up the mess.
> 
> P.
> >
> >
> > and the test then looked like this:
> >
> > static void test_mindistance2d_tolerance(void)
> > {
> > LWGEOM *lw1;
> > lw1 = lwgeom_from_ewkt("LINESTRING(0 0, 1 1)", PARSER_CHECK_NONE);
> > lwgeom_free(lw1);
> > }
> >
> >
> > As I understand it we never free *serialized_lwgeom in LWGEOM_PARSER_RESULT
> >
> > and I think it should be done in lwgeom_from_ewkt after the result are
> > deserialized
> >
> > in the structure LWGEOM_PARSER_RESULT there is also room for a pointer to
> > the LWGEOM version of the geometry, do we use that?
> 
> That's brand new for my parser, which will generate LWGEOM from WKT
> instead of generating serialized forms.
> 
> > I have only tested those things above on trunk r6120 so maybe the parser
> > rewriting is affecting.
> 
> Parser re-writing is causing structures and API to be less than
> consistent. In general the 2.0 pass is hoping to clean up a great deal
> of this so it's a little less full of historical cruft and more
> sensible.
> 
> One major point is to remove all the functions that generate or work
> against serialized form. All API functions (except for two, serialize
> and deserialize) should work against LWGEOM structs. This will allow
> me to (finally, in the very very end) change the serialized form
> underneath the data base to GSERIALIZED.
> 
> Sorry, this is a confusing time to be trying to learn from the API. Be strong.
> 
> Paul
> 
> >
> > I am glad for any hint to help me understand.
> >
> >
> > Thanks
> > Nicklas
> > _______________________________________________
> > postgis-devel mailing list
> > postgis-devel at postgis.refractions.net
> > http://postgis.refractions.net/mailman/listinfo/postgis-devel
> >
> >
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at postgis.refractions.net
> http://postgis.refractions.net/mailman/listinfo/postgis-devel
>