ST_RemEdgeNewFace and ST_RemEdgeModFace crash backend upon not finding side faces

[Lars Aksel Opsahl]

At least in this test case, more info found here ​https://github.com/larsop/resolve-overlap-and-gap/issues/23 .

The problem is the same on

POSTGIS="3.3.0dev 3.1.0alpha2-1416-ged023e8ff" [EXTENSION] PGSQL="120" GEOS="3.9.0-CAPI-1.16.2" PROJ="7.2.1" LIBXML="2.9.10" LIBJSON="0.13.1" LIBPROTOBUF="1.3.3" WAGYU="0.5.0 (Internal)" TOPOLOGY

and

POSTGIS="3.1.1 aaf4c79" [EXTENSION] PGSQL="120" GEOS="3.9.0-CAPI-1.16.2" PROJ="7.2.1" LIBXML="2.9.10" LIBJSON="0.13.1" LIBPROTOBUF="1.3.3" WAGYU="0.5.0 (Internal)" TOPOLOGY

[strk]

Can you provide a stack trace ?

[strk]

Stack trace obtained, from ​https://gitlab.com/nibioopensource/resolve-overlap-and-gap/-/issues/23#note_1257528871:

(gdb) bt
#0  0x0000557650ce5a87 in GetMemoryChunkContext (pointer=0x0)
    at ./build/../src/include/utils/memutils.h:127
#1  pfree (pointer=0x0) at ./build/../src/backend/utils/mmgr/mcxt.c:1033
#2  0x00007f200cb89fb4 in _lwt_RemEdge (topo=topo@entry=0x557651b26210, edge_id=<optimized out>, 
    edge_id@entry=97135, modFace=modFace@entry=0) at lwgeom_topo.c:4052
#3  0x00007f200cb8e37b in lwt_RemEdgeNewFace (topo=topo@entry=0x557651b26210, 
    edge_id=edge_id@entry=97135) at lwgeom_topo.c:4221
#4  0x00007f200cb4a5b0 in ST_RemEdgeNewFace (fcinfo=0x557651b28880) at postgis_topology.c:4633
#5  0x0000557650a40de7 in ExecInterpExpr (state=0x557651b287a8, econtext=0x557651b284d0, 

Happens with no side face of an edge is found in the face table. In normal conditions is should NOT happen because there's a foreign key on the edge table referencing the face table, but as those foreign keys are deferred it can still happen in case of concurrent transactions

[strk]

The hard part will be triggering a failure from testsuite as I suspect it takes a specifically built PostgreSQL version to trigger memory errors.

[strk]

So, specially built PostgreSQL version isn't needed because we're passing 0x0 pointer to pfree and that should ALWAYS segfault. But what I don't understand is how it is possible for ST_RemEdgeNewFace to NOT find any of the faces found in an edge's left_face and right_face columns, as those values *should* theoretically always be checked by foreign key constraints to match existing records in the face table.

I suspect this has to do with the way we use the "read_only" parameter to SPI_execute, that is, has to do with visibility of data changes.

[strk]

I'm able to reproduce the crash but only by dropping the foreign key constraints:

BEGIN;

SELECT topology.CreateTopology('t5106');

INSERT INTO t5106.node VALUES
( 1, NULL, 'POINT(0 0)'::geometry ),
( 2, NULL, 'POINT(10 10)'::geometry );

ALTER TABLE t5106.edge_data DROP constraint left_face_exists;
ALTER TABLE t5106.edge_data DROP constraint right_face_exists;

INSERT INTO t5106.edge VALUES
(
  1, -- edge_id
  1, 2, -- start/end node
  2, -2, -- next left/right edge
  1, 2, -- left/right faces (non existent)
  'LINESTRING(0 0,10 0,10 10)'::geometry
),
(
  2, -- edge_id
  2, 1, -- start/end node
  1, -1, -- next left/right edge
  1, 2, -- left/right faces (non existent)
  'LINESTRING(10 10,0 10,0 0)'::geometry
);

SELECT ST_RemEdgeModFace('t5106', 1);

If I don't drop the left_face_exists and right_face_exists foreign key constraints I cannot create the offending situation

[strk]

I confirm it also happens with RemEdgeModFace

[strk]

Simplified testcase (still needing to DROP foreign key constraints):

SELECT topology.CreateTopology('t5106');

INSERT INTO t5106.node VALUES ( 1, NULL, 'POINT(0 0)' );

-- Cannot reference non-existing faces w/out dropping
-- these constraints
ALTER TABLE t5106.edge_data DROP constraint left_face_exists;
ALTER TABLE t5106.edge_data DROP constraint right_face_exists;

INSERT INTO t5106.edge VALUES
( 
  1, -- edge_id
  1, 1, -- start/end node
  1, -1, -- next left/right edge
  1, 2, -- left/right faces (different, both non-0 and non existent)
  'LINESTRING(0 0,10 0,10 10,0 0)'
);

SELECT 't5106', ST_RemEdgeModFace('t5106', 1);

[strk]

I suspect the original reporter found ST_RemEdgeModFace not segfaulting because that flavor of the version removes LESS faces than ST_RemEdgeNewFace so is less likely to introduce the problem. Or there's really a simpler way to trigger the crash w/out dropping the foreign key constraints and ONLY using RemEdgeNewFace

[Sandro Santilli <strk@…>]

In 9db2a5f/git:

Fix segfault in _lwt_RemEdge when no edge side-faces are found

References #5106 in master branch (3.4.0dev)

Includes regress tests

[Sandro Santilli <strk@…>]

In 69900af/git:

Fix segfault in _lwt_RemEdge when no edge side-faces are found

References #5106 in 3.3 branch (3.3.3dev)

Includes regress tests

[Sandro Santilli <strk@…>]

In e3298c0/git:

Fix segfault in _lwt_RemEdge when no edge side-faces are found

Closes #5106 in 3.2 branch (3.2.5dev)

Includes regress tests