Re: Converting pqsignal to void return

Hi,

On 2025-01-22 10:22:45 -0600, Nathan Bossart wrote:
> On Wed, Jan 22, 2025 at 07:57:52AM -0800, Paul Ramsey wrote:
> > The problem we are having in extension land is that we often run
> > functions in external libraries that take a long time to return, and we
> > would like to ensure that PgSQL users can cancel their queries, even when
> > control has passed into those functions.
> >
> > The way we have done it, historically, has been to take the return value
> > of pqsignal(SIGINT, extension_signint_handler) and remember it, and then,
> > inside extension_signint_handler, call the pgsql handler once we have
> > done our own business..
>
> I see a couple other projects that might be doing something similar [0].
>
> [0] https://codesearch.debian.net/search?q=%3D+pqsignal&literal=1

Grouping by package I see three packages.

All these seem to not handle backend termination, which certainly doesn't seem
optimal - it e.g. means that a fast shutdown won't really work.

1) postgis

For at least some of the cases it doesn't look trivial to convert to
checking QueryCancelPending/ProcDiePending because the signal handler calls
into GEOS and lwgeom.

2) psql-http

Also doesn't handle termination.

Looks like it could trivially be converted to checking
QueryCancelPending/ProcDiePending.

3) timescaledb

This looks to be test only code where the signal handler only prints out a
message. I'd assume it's not critical to log that message.

IOW, the only worrisome case here is postgis.

Given that we are working towards *not* relying on signals for a lot of this,
I wonder if we ought to support registering callbacks to be called on receipt
of query cancellation and backend termination. That then could e.g. call
GEOS_interruptRequest() as postgis does. That'd have a chance of working in a
threaded postgres too - unfortunately it looks like neither lwgeom's nor
GEOS's interrupt mechanisms are thread safe at this point.

It's worth noting that postgis ends up with a bunch of postgres-internals
knowledge due to its desire to handle signals:
https://github.com/postgis/postgis/blob/master/postgis/postgis_module.c#L51-L56

#ifdef WIN32
static void interruptCallback() {
if (UNBLOCKED_SIGNAL_QUEUE())
pgwin32_dispatch_queued_signals();
}
#endif

Which seems really rather undesirable.

But given how windows signals are currently delivered via the equivalent code
in CHECK_FOR_INTERRUPTS(), I don't really see an alternative at this point :(.

Greetings,

Andres Freund