Backend crash with TopoGeo_addLinestring against an invalid topology

[strk]

As reported in ​https://github.com/larsop/resolve-overlap-and-gap/issues/1 there are cases in which an invalid topology triggers a segfault in PostGIS, which assumes some form of validity (existing MBR for faces).

This is probably true since we switched topology from plpgsql to C, and fixes should be backported to all those versions (2.2.0 onward)

[Sandro Santilli <strk@…>]

In ab4f540/git:

Handle corrupted topology in ST_ChangeEdgeGeom

Rather than crashing the backend.
References #4706 in 3.0 branch
Include tests.

[Sandro Santilli <strk@…>]

In 7b9dd285/git:

Handle corrupted topology in ST_ChangeEdgeGeom

Rather than crashing the backend.
References #4706 in 2.5 branch.
Include tests.

[Sandro Santilli <strk@…>]

In a2557be/git:

Handle corrupted topology in ST_ChangeEdgeGeom

Rather than crashing the backend.
References #4706 in master branch.
Include tests.

[Sandro Santilli <strk@…>]

In 5b809e2/git:

Handle corrupted topology in ST_ChangeEdgeGeom

Rather than crashing the backend.
References #4706 in 2.4 branch.
Include tests.

[strk]

I've pushed a fix in branches 2.4, 2.5, 3.0 and master. I realized 2.3 and older are EOLed so won't push there.

[strk]

For the record: the crash was really only in master branch, due to the change introduced in #3221 to allow for GetFaceGeometry to return EMPTY instead of NULL. The test was still good to backport

[robe]

Looks like this broke something

Getting this on berrie (64-bit rasberry pie arm running 32-bit postgresql) and bessie32 (32-bit freebsd)

14:12:55 -----------------------------------------------------------------------------
14:12:55 --- regress/st_changeedgegeom_expected	2020-06-22 10:40:12.551514646 -0700
14:12:55 +++ /tmp/pgis_reg/test_16_out	2020-06-22 11:12:55.631758039 -0700
14:12:55 @@ -39,5 +39,5 @@
14:12:55  T13.2|Edge 29 changed
14:12:55  T13.3|26
14:12:55  ERROR:  Edge motion collision at POINT(-1.1697 47.7825)
14:12:55 -ERROR:  Corrupted topology: face 4, right of edge 7, has no bbox
14:12:55 +ERROR:  Corrupted topology: face 24, right of edge 4, has no bbox
14:12:55  Topology 'city_data' dropped
14:12:55 -----------------------------------------------------------------------------

[strk]

Regina does this only happen in ONE bot ? If so, I guess it depends on some different sorting from queries, resulting in observing one edge instead of another as the first one (edge 7 or edge 4 in this case)

[robe]

It happens on both bessie32 and berrie. Both are 32-bit bots so might be 32-bit related I guess.

[Regina Obe <lr@…>]

In ceca2a8/git:

Take out topology tests that fail on 32-bit references #4706 for Trunk 3.1.0alpha2

[Regina Obe <lr@…>]

In b6f415d3/git:

Took out too much in last commit references #4706 for Trunk 3.1.0alpha2

[strk]

I don't think it is a good idea to keep removing failing tests. It defeats the purpose of tests…

[Algunenano]

I don't think it is a good idea to keep removing failing tests. It defeats the purpose of tests…

I totally agree, but it's also not ok to have some bots broken for 4 weeks. Should we have reverted the change instead until everything could be properly addressed?

[strk]

Rather, the ERROR should be intercepted and references to faces removed (we just want to make sure an exception is thrown, with enough detail, but we don't care about the actual details of face ids)

[robe]

strk feel free to reenable the tests once you've fixed them so that they behave consistently on all platforms and don't give false negatives.

[strk]

I'd be happy to re-enable tests but I would need a way to reproduce the problem. In your report you don't provide a link, can I have a link ? Even better, can I have access to the 32bit machine ? I'd like to look at the built topology via qgis as I really don't even expect the _existance_ of a face with id 24 (highest numbered face has id 11).

I see other tests being disabled in the past, so I'm not sure what's the current state of constructed topology (see [73e5f0092b987b8d65d68d0513cb7a1948c62c25/git])

[Sandro Santilli <strk@…>]

In dcfb125/git:

Re-enable tests for corrupted topology handling

References #4706

[strk]

Regina I've pushed a fix but bessie32 is reported to be offline for pipeline 2​11​0 as I write this comment, from page ​https://debbie.postgis.net/view/PostGIS/job/PostGIS_Worker_Run/label=bessie32/

[robe]

just turned her back on - I was backing up her image in prep for upgrading. I'll trigger a rerun.

[robe]

succeeded

​https://debbie.postgis.net/view/PostGIS/job/PostGIS_Worker_Run/label=bessie32/2112/console