Re: BUG #16378: Invalid memory access on interrupting CLUSTER after CREATE TEMP TABLE

I wrote:
> The difficulty is that the pendingReindexedIndexes list is kept in
> some transaction-local context, so it gets flushed during the transaction
> abort that is the first step of proc_exit processing.  But the static
> pointer to it is still set, causing big problems if we do any system
> catalog accesses later --- like, say, while dropping the session's
> temp tables.

> One idea would be to keep the list in TopMemoryContext, but that feels
> like a band-aid solution.  I think more likely what we ought to do is
> stop trying to use a PG_TRY in reindex_relation to drive cleanup,
> and instead hook ResetReindexPending into transaction abort processing
> honestly.

I spent some more time looking at this.  The "band-aid solution" is no
solution at all; consider the situation where we have some temp tables
and we get a SIGTERM while doing a REINDEX on pg_class.  If we don't
reset the reindex state then our attempts to access pg_class indexes
while clearing out temp tables will fail.

There seem to be two distinct routes to a non-band-aid solution:

1. Instead of PG_TRY, use PG_ENSURE_ERROR_CLEANUP to ensure the reindex
state gets cleared.  This will work since the cleanup code will get
run before any other proc_exit callbacks.  However, the fact that the
reindex state is getting propagated to parallel workers makes me fear
that this solution is inadequate, because the parallel workers won't
have any such cleanup callback.  If they try to do any catalog access
during transaction abort they might have problems.  (Parallel reindex
of a system catalog is mind-bendingly scary anyway, but I don't want
this patch to make it worse.)

2. Clear the reindex state early in transaction abort, as suggested above
and as implemented by the patch below.  This adds a few cycles to xact
abort, which is what the previous coding was trying to avoid, but I think
we are just stuck with doing that.

Thoughts?

			regards, tom lane