Re: segfault in HEAD when too many nested functions call

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: segfault in HEAD when too many nested functions call
Date: 2017-07-26 20:28:38
Message-ID: 25311.1501100918@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2017-07-26 15:03:37 -0400, Tom Lane wrote:
>> Hm, that seems kinda backwards to me; I was envisioning the checks
>> moving to the callees not the callers. I think it'd end up being
>> about the same number of occurrences of CHECK_FOR_INTERRUPTS(),
>> and there would be less of a judgment call about where to put them.

> Hm, that seems a bit riskier - easy to forget one of the places where we
> might need a CFI().

I would argue the contrary. If we put a CFI at the head of each node
execution function, then it's just boilerplate that you copy-and-paste
when you invent a new node type. The way you've coded it here, it
seems to involve a lot of judgment calls. That's very far from being
copy and paste, and the more different it looks from one node type
to another, the easier it will be to forget it.

> We certainly are missing a bunch of them in various nodes

It's certainly possible that there are long-running loops not involving
any ExecProcNode recursion at all, but that would be a bug independent
of this issue. The CFI in ExecProcNode itself can be replaced exactly
either by asking all callers to do it, or by asking all callees to do it.
I think the latter is going to be more uniform and harder to screw up.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-07-26 20:34:18 Re: pg_dump does not handle indirectly-granted permissions properly
Previous Message Andrew Dunstan 2017-07-26 20:15:41 Re: Testlib.pm vs msys