Re: Use of ActiveSnapshot

On 5/14/2007 1:29 PM, Tom Lane wrote:
> Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
>> The comment for the call of pg_plan_queries in util/cache/plancache.c
>> line 469 for example is fatally wrong. Not only should the snapshot be
>> set by all callers at this point, but if the call actually does replan
>> the queries, the existing ActiveSnapshot is replaced with one allocated
>> on the current memory context. If this happens to be inside of a nested
>> SPI call sequence, the innermost SPI stack frame will free the snapshot
>> data without restoring ActiveSnapshot to the one from the caller.
>
> Yeah, I'd been meaning to go back and recheck that point after the code
> settled down, but forgot :-(.
>
> It is possible for RevalidateCachedPlan to be called with no snapshot
> yet set --- at least the protocol Describe messages can do that. I
> don't want Describe to force a snapshot because that would be bad for
> cases like LOCK TABLE at the start of a serializable transaction, so
> RevalidateCachedPlan had better be able to cope with this case.
>
> Since the "typical" case in which no replan is necessary won't touch
> the snapshot, I think we'd better adopt the rule that
> RevalidateCachedPlan never causes any caller-visible change in
> ActiveSnapshot, else we'll be risking very-hard-to-reproduce bugs.
> So my proposal is that RevalidateCachedPlan should set a snapshot for
> itself if it needs to replan and ActiveSnapshot is NULL (else it might
> as well just use the existing snap); and that it should save and restore
> ActiveSnapshot when it does this.

The only problem with that is that there are code paths that set
ActiveSnapshot to palloc()'d memory that is released due to a
MemoryContextDelete() without resetting ActiveSnapshot to NULL. So it
might be possible for RevalidateCachedPlan to go ahead with an
ActiveSnapshot pointing to garbage.

I think it would be cleaner if RevalidateCachedPlan()'s API would have a
Snapshot argument. If it needs a snapshot and the argument is NULL, it
can create (and free) one itself, otherwise it'd use the one given.

Jan

--
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #