Kevin Grittner <kgrittn@ymail.com> writes:
> I propose to push this as it stands except for the postgres_fdw
> part.� The default is easy enough to change if we reach consensus,
> and expanding the scope can be a new patch in a new CF.
> Objections?
There's been enough noise about the external definition that I think
no one has bothered to worry about whether the patch is actually
implemented sanely. I do not think it is: specifically, the notion
that we will call ereport(FATAL) directly from a signal handler
does not fill me with warm fuzzies. While the scope of code in
which the signal is enabled is relatively narrow, that doesn't mean
there's no problem. Consider in particular the case where we are using
SSL: this will mean that we take control away from OpenSSL, which might be
in the midst of doing something if we're unlucky timing-wise, and then
we'll re-entrantly invoke it to try to send an error message to the
client. That seems pretty unsafe. Another risky flow of control is
if ReadCommand throws an ordinary error and then the timeout interrupt
happens while we're somewhere in the transaction abort logic (the
sigsetjmp stanza in postgres.c).
I'd be happier if this were implemented in the more traditional
style where the signal handler just sets a volatile flag variable,
which would be consulted at determinate places in the mainline logic.
Or possibly it could be made safe if we only let it throw the error
directly when ImmediateInterruptOK is true (compare the handling
of notify/catchup interrupts).
regards, tom lane
