free objects in the right context (#139)

* free objects in the right context

* remove get_context

* add get_context

* probe ci

* fix

* fix

* fix STRtree memory leak

* move STRtree docstring

Co-authored-by: Martijn Visser <[email protected]>

Author: jw3126

src/geos_functions.jl

@@ -477,8 +477,11 @@ function cloneGeom(ptr::GEOSGeom, context::GEOSContext = _context)
     result
 end
 
-destroyGeom(ptr::GEOSGeom, context::GEOSContext = _context) =
-    GEOSGeom_destroy_r(context.ptr, ptr)
+function destroyGeom(ptr::GEOSGeom, context::GEOSContext = _context)
+    GC.@preserve context begin
+        GEOSGeom_destroy_r(context.ptr, ptr)
+    end
+end
 
 # -----
 # Topology operations - return NULL on exception.
@@ -797,8 +800,11 @@ function prepareGeom(ptr::GEOSGeom, context::GEOSContext = _context)
     result
 end
 
-destroyPreparedGeom(ptr::Ptr{GEOSPreparedGeometry}, context::GEOSContext = _context) =
-    GEOSPreparedGeom_destroy_r(context.ptr, ptr)
+function destroyPreparedGeom(ptr::Ptr{GEOSPreparedGeometry}, context::GEOSContext = _context)
+    GC.@preserve context begin
+        GEOSPreparedGeom_destroy_r(context.ptr, ptr)
+    end
+end
 
 function prepcontains(
     g1::Ptr{GEOSPreparedGeometry},

src/geos_types.jl

@@ -2,11 +2,12 @@ abstract type AbstractGeometry end
 
 mutable struct Point <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a point from a pointer - only makese sense if it is a pointer to a point, otherwise error
     function Point(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         point = if id == GEOS_POINT
-            point = new(cloneGeom(ptr, context))
+            point = new(cloneGeom(ptr, context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a point (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -25,16 +26,19 @@ end
 
 mutable struct MultiPoint <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a multipoint from a pointer - only makes sense if it is a pointer to a multipoint
     # or to a point, otherwise error
     function MultiPoint(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         multipoint = if id == GEOS_MULTIPOINT
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         elseif id == GEOS_POINT
             new(createCollection(GEOS_MULTIPOINT, 
                                  GEOSGeom[cloneGeom(ptr, context)],
-                                 context))
+                                 context), 
+                context
+               )
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a multipoint (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -62,11 +66,12 @@ end
 
 mutable struct LineString <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a linestring from a linestring pointer, otherwise error
     function LineString(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         line = if id == GEOS_LINESTRING
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a linestring (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -76,23 +81,24 @@ mutable struct LineString <: AbstractGeometry
     end
     #create a linestring from a list of coordiantes
     function LineString(coords::Vector{Vector{Float64}}, context::GEOSContext = _context)
-        line = new(createLineString(coords, context))
+        line = new(createLineString(coords, context), context)
         finalizer(destroyGeom, line)
         line
     end
 end
 
 mutable struct MultiLineString <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a multiline string from a multilinestring or a linestring pointer, else error
     function MultiLineString(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         multiline = if id == GEOS_MULTILINESTRING
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         elseif id == GEOS_LINESTRING
             new(createCollection(GEOS_MULTILINESTRING, 
                                  GEOSGeom[cloneGeom(ptr, context)], 
-                                 context))
+                                 context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a multi-linestring (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -112,11 +118,12 @@ end
 
 mutable struct LinearRing <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a linear ring from a linear ring pointer, otherwise error
     function LinearRing(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         ring = if id == GEOS_LINEARRING
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a linear ring (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -127,7 +134,7 @@ mutable struct LinearRing <: AbstractGeometry
     # create linear ring from a list of coordinates - 
     # first and last coordinates must be the same
     function LinearRing(coords::Vector{Vector{Float64}}, context::GEOSContext = _context)
-        ring = new(createLinearRing(coords, context))
+        ring = new(createLinearRing(coords, context), context)
         finalizer(destroyGeom, ring)
         ring
     end
@@ -136,13 +143,14 @@ end
 
 mutable struct Polygon <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create polygon using GEOSGeom pointer - only makes sense if pointer points to a polygon or a linear ring to start with. 
     function Polygon(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         polygon = if id == GEOS_POLYGON
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         elseif id == GEOS_LINEARRING
-            new(cloneGeom(createPolygon(ptr, context), context))
+            new(cloneGeom(createPolygon(ptr, context), context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a polygon (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -155,7 +163,7 @@ mutable struct Polygon <: AbstractGeometry
     function Polygon(coords::Vector{Vector{Vector{Float64}}}, context::GEOSContext = _context)
         exterior = createLinearRing(coords[1], context)
         interiors = GEOSGeom[createLinearRing(lr, context) for lr in coords[2:end]]
-        polygon = new(createPolygon(exterior, interiors, context))
+        polygon = new(createPolygon(exterior, interiors, context), context)
         finalizer(destroyGeom, polygon)
         polygon
     end
@@ -173,16 +181,19 @@ end
 
 mutable struct MultiPolygon <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create multipolygon using a multipolygon or polygon pointer, else error
     function MultiPolygon(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         multipolygon = if id == GEOS_MULTIPOLYGON
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         elseif id == GEOS_POLYGON
             new(createCollection(
                     GEOS_MULTIPOLYGON,
                     GEOSGeom[cloneGeom(ptr, context)],
-                    context))
+                    context),
+                context
+               )
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a multi-polygon (yet).
                    Please open an issue if you think this conversion makes sense.")    
@@ -217,11 +228,12 @@ end
 
 mutable struct GeometryCollection <: AbstractGeometry
     ptr::GEOSGeom
+    context::GEOSContext
     # create a geometric collection from a pointer to a geometric collection, else error
     function GeometryCollection(ptr::GEOSGeom, context::GEOSContext = _context)
         id = LibGEOS.geomTypeId(ptr, context)
         geometrycollection = if id == GEOS_GEOMETRYCOLLECTION
-            new(cloneGeom(ptr, context))
+            new(cloneGeom(ptr, context), context)
         else
             error("LibGEOS: Can't convert a pointer to an element with a GeomType ID of $id to a geometry collection (yet).
                    Please open an issue if you think this conversion makes sense.")
@@ -250,7 +262,9 @@ const Geometry = Union{
     GeometryCollection,
 }
 
-function destroyGeom(obj::Geometry, context::GEOSContext = _context)
+get_context(obj::Geometry) = obj.context
+function destroyGeom(obj::Geometry)
+    context = get_context(obj)
     destroyGeom(obj.ptr, context)
     obj.ptr = C_NULL
 end
@@ -260,7 +274,10 @@ mutable struct PreparedGeometry{G<:AbstractGeometry} <: AbstractGeometry
     ownedby::G
 end
 
-function destroyGeom(obj::PreparedGeometry, context::GEOSContext = _context)
+get_context(obj::PreparedGeometry) = get_context(obj.ownedby)
+
+function destroyGeom(obj::PreparedGeometry)
+    context = get_context(obj)
     destroyPreparedGeom(obj.ptr, context)
     obj.ptr = C_NULL
 end

src/strtree.jl

@@ -1,8 +1,3 @@
-struct STRtree{T}
-    ptr::Ptr{GEOSSTRtree}  # void pointer to the tree
-    items::T  # any geometry for which an envelope can be derived
-end
-
 """
     STRtree(items; nodecapacity=10, context::GEOSContext=_context)
 
@@ -15,13 +10,31 @@ envelope is defined. The tree cannot be changed after its creation.
 - `context`: The GEOS context in which the tree should be created in. Defaults to the global
   LibGEOS context.
 """
-function STRtree(items; nodecapacity = 10, context::GEOSContext = _context)
-    tree = LibGEOS.GEOSSTRtree_create_r(context.ptr, nodecapacity)
-    for item in items
-        envptr = envelope(item).ptr
-        GEOSSTRtree_insert_r(context.ptr, tree, envptr, pointer_from_objref(item))
+mutable struct STRtree{T}
+    ptr::Ptr{GEOSSTRtree}  # void pointer to the tree
+    items::T  # any geometry for which an envelope can be derived
+    context::GEOSContext
+    function STRtree(items; nodecapacity = 10, context::GEOSContext = _context)
+        tree = LibGEOS.GEOSSTRtree_create_r(context.ptr, nodecapacity)
+        for item in items
+            envptr = envelope(item).ptr
+            GEOSSTRtree_insert_r(context.ptr, tree, envptr, pointer_from_objref(item))
+        end
+        T = typeof(items)
+        ret = new{T}(tree, items, context)
+        finalizer(destroySTRtree, ret)
+        return ret
     end
-    return STRtree(tree, items)
+end
+
+get_context(obj::STRtree) = obj.context
+
+function destroySTRtree(obj::STRtree)
+    context = get_context(obj)
+    GC.@preserve context begin
+        GEOSSTRtree_destroy_r(context.ptr, obj.ptr)
+    end
+    obj.ptr = C_NULL
 end
 
 """
@@ -37,7 +50,7 @@ Returns the objects within `tree`, whose envelope intersects the envelope of `ge
 function query(
     tree::STRtree{T},
     geometry::Geometry;
-    context::GEOSContext = _context,
+    context::GEOSContext = get_context(tree),
 ) where {T}
     matches = eltype(T)[]
 
@@ -50,12 +63,14 @@ function query(
     end
 
     cfun_callback = @cfunction($callback, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}))
-    GEOSSTRtree_query_r(
-        context.ptr,
-        tree.ptr,
-        geometry.ptr,
-        cfun_callback,
-        pointer_from_objref(matches),
-    )
+    GC.@preserve context tree geometry matches begin
+        GEOSSTRtree_query_r(
+            context.ptr,
+            tree.ptr,
+            geometry.ptr,
+            cfun_callback,
+            pointer_from_objref(matches),
+        )
+    end
     return matches
 end

test/test_geos_types.jl

@@ -350,4 +350,23 @@ testValidTypeDims(multipoly::LibGEOS.MultiPolygon) =
         LibGEOS.destroyGeom(geomcollect_ctx)
     end
 
-end
+end
+
+@testset "Multi threading" begin
+    function f91(n)
+        # adapted from https://github.com/JuliaGeo/LibGEOS.jl/issues/91#issuecomment-1267732709
+        contexts = [LibGEOS.GEOSContext() for i=1:Threads.nthreads()]
+        p = [[[-1.,-1],[+1,-1],[+1,+1],[-1,+1],[-1,-1]]]
+        Threads.@threads :static for i=1:n
+            ctx = contexts[Threads.threadid()]
+            g1 = LibGEOS.Polygon(p, ctx)
+            g2 = LibGEOS.Polygon(p, ctx)
+            for j=1:n
+                LibGEOS.intersects(g1.ptr, g2.ptr, ctx)
+            end
+        end
+        GC.gc(true)
+        return nothing
+    end
+    f91(91)
+end