[postgis-tickets] [PostGIS] #3407: topology: crash when using ST_AddEdgeModFace on face that's part of multiple topogeometries

[PostGIS]

#3407: topology: crash when using ST_AddEdgeModFace on face that's part of
multiple topogeometries
----------------------+---------------------------
 Reporter:  rulus     |      Owner:  strk
     Type:  defect    |     Status:  new
 Priority:  medium    |  Milestone:  PostGIS 2.2.1
Component:  topology  |    Version:  2.2.x
 Keywords:            |
----------------------+---------------------------
 When using ST_AddEdgeModFace to add a new edge splitting a face that's
 part of more than 1 topogeometry, postgres crashes with a segfault.

 I am running version
 PostgreSQL 9.4.5 on x86_64-unknown-linux-gnu, compiled by gcc (Debian
 5.2.1-23) 5.2.1 20151028, 64-bit POSTGIS="2.2.0 r14208"
 GEOS="3.5.0-CAPI-1.9.0 r4084" PROJ="Rel. 4.9.2, 08 September 2015"
 GDAL="GDAL 2.0.1, released 2015/09/15" LIBXML="2.9.3" LIBJSON="0.11.99"
 TOPOLOGY RASTER

 Using gdb I got this backtrace:
 {{{
 Program received signal SIGSEGV, Segmentation fault.
 0x00007fd5ac1d8358 in cb_updateTopoGeomFaceSplit (topo=0x55c79d7b6ae0,
 split_face=<optimized out>, new_face1=133, new_face2=-1) at
 postgis_topology.c:2018
 2018        HeapTuple row = SPI_tuptable->vals[i];
 (gdb) bt
 #0  0x00007fd5ac1d8358 in cb_updateTopoGeomFaceSplit (topo=0x55c79d7b6ae0,
 split_face=<optimized out>, new_face1=133, new_face2=-1) at
 postgis_topology.c:2018
 #1  0x00007fd5ac201cfd in lwt_be_updateTopoGeomFaceSplit
 (topo=0x55c79d7b6b48, topo=0x55c79d7b6b48, new_face2=-1, new_face1=133,
 split_face=<optimized out>) at lwgeom_topo.c:350
 #2  _lwt_AddEdge (topo=topo at entry=0x55c79d7b6b48,
 start_node=start_node at entry=142, end_node=end_node at entry=208,
 geom=geom at entry=0x55c79d7b46f0, skipChecks=skipChecks at entry=0,
 modFace=modFace at entry=1)
     at lwgeom_topo.c:2696
 #3  0x00007fd5ac20500b in lwt_AddEdgeModFace
 (topo=topo at entry=0x55c79d7b6b48, start_node=start_node at entry=142,
 end_node=end_node at entry=208, geom=geom at entry=0x55c79d7b46f0,
 skipChecks=skipChecks at entry=0)
     at lwgeom_topo.c:2722
 #4  0x00007fd5ac1d9f20 in ST_AddEdgeModFace (fcinfo=0x55c79d7df650) at
 postgis_topology.c:3207
 #5  0x000055c79cd5ee83 in ?? ()
 #6  0x000055c79cd64e6d in ExecProject ()
 #7  0x000055c79cd77568 in ExecResult ()
 #8  0x000055c79cd5dd18 in ExecProcNode ()
 #9  0x000055c79cd5af5e in standard_ExecutorRun ()
 #10 0x000055c79cd8188f in ?? ()
 #11 0x000055c79cd81bac in SPI_execute_plan_with_paramlist ()
 #12 0x00007fd5b36094a4 in ?? () from
 /usr/lib/postgresql/9.4/lib/plpgsql.so
 #13 0x00007fd5b360d6c5 in ?? () from
 /usr/lib/postgresql/9.4/lib/plpgsql.so
 #14 0x00007fd5b360faad in ?? () from
 /usr/lib/postgresql/9.4/lib/plpgsql.so
 #15 0x00007fd5b360fcd2 in plpgsql_exec_function () from
 /usr/lib/postgresql/9.4/lib/plpgsql.so
 #16 0x00007fd5b3604d49 in plpgsql_call_handler () from
 /usr/lib/postgresql/9.4/lib/plpgsql.so
 #17 0x000055c79cd5ee83 in ?? ()
 #18 0x000055c79cd64e6d in ExecProject ()
 #19 0x000055c79cd77568 in ExecResult ()
 #20 0x000055c79cd5dd18 in ExecProcNode ()
 #21 0x000055c79cd5af5e in standard_ExecutorRun ()
 #22 0x000055c79ce55247 in ?? ()
 #23 0x000055c79ce56888 in PortalRun ()
 #24 0x000055c79ce534e3 in PostgresMain ()
 #25 0x000055c79cc1c773 in ?? ()
 #26 0x000055c79cdfd50a in PostmasterMain ()
 #27 0x000055c79cc1d90c in main ()
 }}}

 Leading me to the cb_updateTopoGeomFaceSplit function in
 topology/postgis_topology.c (I used the 2.2.0 release from the git tag).

 The problem seems to occur when the first SELECT query yields > 1 result,
 in which case {{{ntopogeoms}}} > 1 and the following loop is executed
 multiple times.

 Inside the loop the global SPI_tuptable variable is used to access the
 results from the SELECT query. However, inside the loop other INSERT
 queries are issued overwriting this SPI_tuptable variable causing the
 crash in the second iteration.

 I tried a proof-of-concept fix by saving a reference to the original
 SPI_tuptable and this seems to fix the crash and behave correctly:
 {{{
 --- a/topology/postgis_topology.c
 +++ b/topology/postgis_topology.c
 @@ -1975,6 +1975,7 @@ cb_updateTopoGeomFaceSplit ( const LWT_BE_TOPOLOGY*
 topo,
    StringInfoData sqldata;
    StringInfo sql = &sqldata;
    int i, ntopogeoms;
 +  SPITupleTable *tupTable;
    const char *proj = "r.element_id, r.topogeo_id, r.layer_id,
 r.element_type";

    POSTGIS_DEBUGF(1, "cb_updateTopoGeomFaceSplit signalled "
 @@ -2013,10 +2014,11 @@ cb_updateTopoGeomFaceSplit ( const
 LWT_BE_TOPOLOGY* topo,
    }

    ntopogeoms = SPI_processed;
 +  tupTable = SPI_tuptable;
    for ( i=0; i<ntopogeoms; ++i )
    {
 -    HeapTuple row = SPI_tuptable->vals[i];
 -    TupleDesc tdesc = SPI_tuptable->tupdesc;
 +    HeapTuple row = tupTable->vals[i];
 +    TupleDesc tdesc = tupTable->tupdesc;
      int negate;
      int element_id;
      int topogeo_id;
 }}}

--
Ticket URL: <https://trac.osgeo.org/postgis/ticket/3407>
PostGIS <http://trac.osgeo.org/postgis/>
The PostGIS Trac is used for bug, enhancement & task tracking, a user and developer wiki, and a view into the subversion code repository of PostGIS project.