Peter Geoghegan wrote:
> On Mon, Apr 16, 2018 at 10:09 AM, Andres Freund <andres@anarazel.de> wrote:
> >> I have vague memories of somebody submitting a version of this code
> >> that returned approximate answers, good enough for monitoring ...
> >
> > That might have been me, but I don't recall the details anymore...
>
> Obviously you're thinking of 6e654546fb61f62cc982d0c8f62241b3b30e7ef8.
> I have a hard time imagining how that could be implicated in this hard
> crash, though, except perhaps by removing something that masked the
> problem in earlier versions.
Yeah, I wasn't commenting on the crash itself -- just on how bad it is
to let Zabbix monitor your database in this way. Maybe it *is* useful
in certain situations, as Andres says, but I bet zabbix doesn't actually
discriminate like that.
Now, looking at the code
for (i = 0; i < node->nfuncs; i++)
{
FunctionScanPerFuncState *fs = &node->funcstates[i];
if (fs->func_slot)
ExecClearTuple(fs->func_slot);
if (fs->tstore != NULL)
{
tuplestore_end(node->funcstates[i].tstore);
fs->tstore = NULL;
}
and tuplestore_end does this:
if (state->myfile)
BufFileClose(state->myfile);
without setting anything in state to NULL; so we're relying on the
caller fs->tstore to null to avoid repeated tuplestore_end calls. I
can't see any way for this to misbehave, but maybe the funcstate appears
more than once in the PerFuncState array, and we clean it correctly the
first time around and then invoke the tuplestore_end() the second time
to the memory that was previously freed? I think this makes no sense
unless we share FunctionScanPerFuncState elements -- do we?
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
