[geos-devel] An Immodest Proposal

[Mark Cave-Ayland]

On Mon, 2008-09-29 at 13:42 -0700, Paul Ramsey wrote:
> All,
> 
> We are currently grappling with a pretty big issue in the PostGIS/GEOS
> integration, namely that PgSQL allocates it's memory inside pools
> (palloc/pfree), while GEOS allocates in general memory (malloc/free).
> This (a) imposes a requirement of memory management perfection on
> GEOS, otherwise the long-running postgres processes will leak and (b)
> can lead to insoluble problems in certain implementation patterns.
> 
> We have recently found one such problem pattern. When using
> preparedgeometry in postgis, we create a GEOS geometry and associated
> prepared geometry, these are duly malloc'ed, but we store references
> to them in a palloc'ed struct in a memory pool which lasts for the
> life of a postgres query.  All is good (well, there are memory leaks
> in the prepared stuff still, but those are theoretically fixable)
> until the end of the query, when postgres cleans up the query memory
> pool. All of a sudden the struct with the references to the geometry
> and prepared geometry are gone -- but the objects have not been freed.
> 
> Basically, the prepared geometry system will leak one geometry and one
> prepared geometry for every SQL statement run, and there's SFA we can
> do about it.
> 
> My immodest proposal is to use geosInit as a location where users can
> provide their own allocator/deallocators.  Right now, geosInit lets us
> set custom error handlers, so this extends the idea further.
> 
> In GEOS we would override new/delete in GEOS to our own functions that
> would check and see if the custom alloc/dealloc functions were set. If
> not, they would fall back to malloc/free, otherwise use what was
> provided.  For postgis, we would provide palloc/pfree in our geosInit
> calls.
> 
> In this way, we could hook GEOS into the PgSQL pool system at runtime,
> while still have it operate in the normal way when used with other
> applications. And other applications with their own memory systems
> could also more tightly integrate GEOS.
> 
> Thoughts?
> 
> P.

Well, in terms of providing a quick solution that works with PostgreSQL
then it can be seen as an improvement. But the fundamental issue is
still that GEOS still leaks memory, and nearly every other application
will still suffer the same consequences. Providing a hook so that other
applications can handle the context tracking and lack of memory lifetime
of a library which is known to leak is not really a suitable solution
IMO.

At the end of the day, for the solution to work 100% it still requires
someone to go through the code and specify the lifetime for each "new"
constructor in terms of a hierarchical memory system. I guess it depends
on how desperately do people want a new GEOS release? I'd argue that
there should be a new 3.0 branch release which has the containment fixes
real soon now, but if there is time for 3.1, it would be well worth
someone spending the time on this to nail the issue for once and for
all. In terms of effort required, it would be more repetitive than
technical.


ATB,

Mark.