Re: BUG #15378: SP-GIST memory context screwup?

On Tue, Sep 11, 2018 at 6:13 AM Andrew Gierth <andrew@tao11.riddles.org.uk>
wrote:

> [CCing Teodor as committer of ccd6eb49a]
>
> >>>>> "PG" == PG Bug reporting form <noreply@postgresql.org> writes:
>
>  PG> I found this while analyzing a report from IRC that initially
>  PG> looked like a PostGIS bug, but which I now think is breakage in
>  PG> spgist:
>
>  PG> spgrescan starts out by doing
>  PG>     MemoryContextReset(so->traversalCxt);
>
>  PG> then later it calls resetSpGistScanOpaque(so);
>  PG> which calls freeScanStack(so)
>  PG> which calls freeScanStackEntry(so)
>  PG> which does:
>
>  PG>     if (stackEntry->traversalValue)
>  PG>         pfree(stackEntry->traversalValue);
>
>  PG> But stackEntry->traversalValue, if not NULL, is supposed to have
>  PG> been allocated in so->traversalCxt, and so it's already gone.
>  [...]
>  PG> Unfortunately I don't think this can be demonstrated with the
>  PG> built-in spgist opclasses, which don't allocate traversalValues.
>
> Turns out I was looking in the wrong place, and in fact we can reproduce
> this easily (at least on an assert build):
>
> create table boxes (b box);
> insert into boxes
>   select box(point(i,j),point(i+s,j+s))
>     from generate_series(1,100,5) i,
>          generate_series(1,100,5) j,
>          generate_series(1,10) s;
> create index on boxes using spgist (b);
> select *
>   from (values
> (box(point(5,5),point(8,8))),(box(point(2,2),point(12,12)))) v(b)
>        cross join lateral (select * from boxes where boxes.b && v.b limit
> 1) pt;
>
> So this logic was added in ccd6eb49a and was wrong from the start.
> Testing suggests that removing the offending pfree does indeed fix the
> issue; any objections?
>

No objections from me.  What about
moving MemoryContextReset(so->traversalCxt) into resetSpGistScanOpaque()?
For me it seems that resetting of traversal memory context is part of
opaque reset.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company