Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#3463 closed defect (fixed)

Crash loading polygons in topology

Reported by: strk Owned by: strk
Priority: blocker Milestone: PostGIS 2.2.2
Component: topology Version: 2.2.x
Keywords: Cc: esseffe

Description

The attached shapefile, loaded in a topology, triggers a backend crash. Steps to reproduce:

unzip sezioni.zip
shp2pgsql -S sezionicrash.shp | psql
psql -c "select topogeo_addpolygon('sezionicrash', geom) from sezionicrash"

Attachments (1)

sezioni.zip (41.8 KB) - added by strk 6 years ago.

Download all attachments as: .zip

Change History (16)

Changed 6 years ago by strk

Attachment: sezioni.zip added

comment:1 Changed 6 years ago by strk

Cc: esseffe added

Valgrind report from an equivalent spatialite run (by Alessandro Furieri):

==3583== Process terminating with default action of signal 11 (SIGSEGV)
==3583==  Access not within mapped region at address 0x0 
==3583==    at 0x7C6E230: lwgeom_is_empty (lwgeom.c:1303)
==3583==    by 0x7C6CFF9: lwgeom_add_bbox (lwgeom.c:602)
==3583==    by 0x7CB8D41: lwt_ChangeEdgeGeom (lwgeom_topo.c:3579)
==3583==    by 0x7CBCBBC: lwt_AddPoint (lwgeom_topo.c:5230)
==3583==    by 0x7CBD0A8: _lwt_AddLineEdge (lwgeom_topo.c:5403)
==3583==    by 0x7CBDE34: lwt_AddLine (lwgeom_topo.c:5791)
==3583==    by 0x6F88506: gaiaTopoGeo_AddLineString (gaia_auxtopo.c:5219)
==3583==    by 0x6F8A56D: auxtopo_insert_into_topology (gaia_auxtopo.c:5659)
==3583==    by 0x6F8B5D4: do_FromGeoTableExtended_block (gaia_auxtopo.c:5933)                     
==3583==    by 0x6F8BD33: gaiaTopoGeo_FromGeoTableExtended (gaia_auxtopo.c:6056)
==3583==    by 0x6F6A620: fnctaux_TopoGeo_FromGeoTableExt (gaia_topology.c:4032)                     
==3583==    by 0x6E1F555:  fnct_TopoGeo_FromGeoTableExt (spatialite.c:35065)

comment:2 Changed 6 years ago by strk

The bug is in unhandled NULL return from lwt_GetFaceGeometry within lwt_ChangeEdgeGeom:

 /*                                                                            
  -- Update faces MBR of left and right faces                                   
  -- TODO: think about ways to optimize this part, like see if                  
  --       the old edge geometry partecipated in the definition                 
  --       of the current MBR (for shrinking) or the new edge MBR               
  --       would be larger than the old face MBR...                             
  --                                                                            
  */                                                                            
  int facestoupdate = 0;                                                        
  LWT_ISO_FACE faces[2];                                                        
  LWGEOM *nface1 = NULL;                                                        
  LWGEOM *nface2 = NULL;                                                        
  if ( oldedge->face_left != 0 )   
  {      
    nface1 = lwt_GetFaceGeometry(topo, oldedge->face_left);  
    lwgeom_add_bbox(nface1);    /// <--- nface1 is NULL here

comment:3 Changed 6 years ago by strk

Adding handling for the case prints:

ERROR:  Corrupted topology: edges of face 37 (5 edges) do not form a polygon

Face 37 is fine before starting the new load, but it is very narrow. With an area of 0.0186 units and 2 almost parallel bounding edges with an angle on the shared starting node of < 0.0002 degrees.

comment:4 Changed 6 years ago by strk

BuildArea? algorithm is used to build the area out of the edges, and is the one returning NULL. Should come straight from the GEOS polygonizer. I guess there's a full collapse somewhere, thus the edge changing attempt should be invalid, and reported as such.

comment:5 Changed 6 years ago by strk

This is the set of edges that BuildArea? is incapable of getting an area from (will get the HEXWKB form in a subsequent comment):

NOTICE: Could not build an area with edges: MULTILINESTRING((634451.780306051602 4829559.16259394772,634451.780323039857 4829559.16248522233,634469.462315359502 4829541.48048062716),(634451.780306051602 4829559.16259394772,634485.780000000261 4829525.16290000081,634488.637000000104 4829521.20830000006,634506.460690993699 4829496.52791544143),(634469.462315359502 4829541.48048062716,634485.780000000261 4829525.16269999929,634485.781473573064 4829525.16065966897,634506.460690993699 4829496.52791544143),(634451.780299999751 4829559.16259999946,634469.462315359502 4829541.48048062716),(634451.780299999751 4829559.16259999946,634451.780306051602 4829559.16259394772))

comment:6 Changed 6 years ago by strk

The edge collection HEXWKB:

0105000000050000000102000000030000005946848FA75C234172F067CA5D6C52416180868FA75C23416B2866CA5D6C52414B99B4ECCA5C2341D131C05E596C52410102000000040000005946848FA75C234172F067CA5D6C5241F8285C8FEB5C234120F46C4A556C524130DD2446F15C234186C9544D546C52419FB0DFEB145D2341D95DC9214E6C52410102000000040000004B99B4ECCA5C2341D131C05E596C5241F8285C8FEB5C234142AD694A556C5241DE4D1D90EB5C23417E3F484A556C52419FB0DFEB145D2341D95DC9214E6C5241010200000002000000487B838FA75C2341D40968CA5D6C52414B99B4ECCA5C2341D131C05E596C5241010200000002000000487B838FA75C2341D40968CA5D6C52415946848FA75C234172F067CA5D6C5241

I confirm ST_BuildArea returns NULL with that input, while ST_BuildArea(ST_Node(x)) returns an area.

comment:7 Changed 6 years ago by strk

For the record: topology in 2.1 branch does not crash nor does it throw an exception, ValidateTopology? returns no errors after load (but we know it doesn't check edge linking) and the resulting topology has 500 nodes, 992 edges, 492 faces.

comment:8 Changed 6 years ago by strk

min/max/avg area of the topology faces as loaded in 2.1 branch:

min | 3.18838413098732e-09
max | 8731603.80629115
avg | 49514.481442308

There are 4 faces with area < 1e-8, 13 with area < 1e-5

comment:9 Changed 6 years ago by strk

Simplified test, self-contained:

SELECT DropTopology('bug3463');                                                 
SELECT CreateTopology('bug3463');                                               
SELECT TopoGeo_AddLinestring('bug3463', g) FROM ( VALUES                        
('LINESTRING(634451.7803 4829559.1626,634485.78 4829525.1627,634506.460690994 4829496.52791544)'::geometry),
('LINESTRING(634447.034152733 4829589.53826525,634451.7794 4829559.1635,634451.7803 4829559.1626)'),
('LINESTRING(634506.460690994 4829496.52791544,634511.7798 4829489.163,634516.720831369 4829468.57536243)'),
('LINESTRING(634451.7803 4829559.1626,634485.78 4829525.1629,634488.637 4829521.2083,634506.460690994 4829496.52791544)'),
('LINESTRING(634506.460690994 4829496.52791544,634511.7792 4829489.1634,634516.720831369 4829468.57536243)')
) foo (g);                                                                      
                                                                                
SELECT TopoGeo_AddLinestring('bug3463',                                         
 'LINESTRING(634452.780338203 4829623.16197433,634446.780374345 4829591.16223176,634451.78032304 4829559.16248522,634485.780040461 4829525.1627442,634488.530017153 4829521.16277512,634511.779820754 4829489.16302201,634523.779707286 4829439.16341655,632187.797669813 4827723.17794488,631718.05142978 4827788.42759019,631718.05144314 4827826.17728887,632759.754208638 4830776.16598454)'
::geometry); 

comment:10 Changed 6 years ago by strk

The self-contained test also fails with pg21, but there it correctly throws an exception: SQL/MM Spatial exception - geometry crosses edge 4

comment:11 Changed 6 years ago by strk

(In [14666]) Fix crash on face-collapsing edge change

See #3463

comment:12 Changed 6 years ago by strk

r14666 turns the crash into an exception (trunk/2.3.0dev)

comment:13 Changed 6 years ago by strk

(In [14667]) Fix crash on face-collapsing edge change

See #3463

comment:14 Changed 6 years ago by strk

Resolution: fixed
Status: newclosed

r14667 backports the crash fix to 2.2 branch (2.2.2). I'm closing this as the crash is fixed. Will file a followup for the exception being thrown.

comment:15 Changed 6 years ago by strk

#3464 is the continuation of this ticket.

Note: See TracTickets for help on using tickets.