OSGeo / gdal

GDAL is an open source MIT licensed translator library for raster and vector geospatial data formats.
https://gdal.org
Other
4.79k stars 2.51k forks source link

Python bindings: Feature.GetGeometryRef() on a temporary Python feature object doesn't keep the C++ feature object alive #4401

Open ktaylora opened 3 years ago

ktaylora commented 3 years ago

Expected behavior and actual behavior.

When repairing broken geometries in Python, we make use of ogr's IsValid() and MakeValid() functions in our application code. When checking for invalid geometries, I am encountering a segfault. When I arbitrarily apply MakeValid() to the same geometries and then check for validity with IsValid(), the geometries are repaired and OGR behaves as expected. This isn't a critical bug, because we can just always call MakeValid() first on geometries we suspect are broken (see below), but it would be nice to LBYL without a segfault.

Steps to reproduce the problem.

using the following geopackage containing broken geometries that need repair.

In [1]: from osgeo import ogr
In [2]: gpkg_connection = ogr.Open("GAZ_35_New_Mexico_GU_STATEORTERRITORY.gpkg",0)
In [3]: gpkg_connection.GetLayerByIndex(0).GetName()
Out[3]: 'clippoly'
In [4]: gpkg_connection.GetLayerByIndex(0).GetFeature(1).GetGeometryRef().IsValid()
Segmentation fault
In [3]: from osgeo import ogr
In [4]: gpkg_connection = ogr.Open('GAZ_35_New_Mexico_GU_STATEORTERRITORY.gpkg', 0)
In [5]: clippoly = gpkg_connection.GetLayerByIndex(0)
In [6]: clippoly.GetFeatureCount()
Out[6]: 1
In [7]: feat = clippoly.GetFeature(1)
In [8]: geom = feat.GetGeometryRef()
In [9]: geom = geom.MakeValid()
In [10]: geom.IsValid()
Out[10]: True

Operating system

Linux *** 4.14.238-182.422.amzn2.x86_64 #1 SMP *** UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

GDAL version and provenance

GDAL (and dependencies) were compiled from source using gcc version 7.3.1 20180712 (Red Hat 7.3.1-13) (GCC). Relevant dependencies (and versions) :

GDAL_VERSION=3.3.1
SQLITE_VERSION=3350500
CMAKE_VERSION=3.12.0
POSTGRES_VERSION=11.12
PYTHON_VERSION=3.8.10
SPATIAL_INDEX_VERSION=1.9.3
FGDB_API_VERSION=1.5.1
GEOS_VERSION=3.8.1

./configure PQ_CFLAGS="-I/usr/include" PQ_LIBS="-L/usr/lib -lpq" --prefix=/usr \
--with-python \
--with-fgdb=/usr/local/filegdb_api \
--with-pg \
--with-sqlite3 \
--with-geos
rouault commented 3 years ago

You've hit on of the https://gdal.org/api/python_gotchas.html Change gpkg_connection.GetLayerByIndex(0).GetFeature(1).GetGeometryRef().IsValid() to f = gpkg_connection.GetLayerByIndex(0).GetFeature(1) f.GetGeometryRef().IsValid()

ktaylora commented 3 years ago

Thanks, Even. I shoulda Googled it.

rouault commented 3 years ago

I'll reopen that one as I believe we could fix that without breaking existing code (contrary to reference counting on dataset objects)