Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Noah Misch <noah(at)leadboat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Is RecoveryConflictInterrupt() entirely safe in a signal handler?
Date: 2023-01-14 02:23:11
Message-ID: CA+hUKGJtvowY_HWfHriU1D16i5EmyqXeBUvUC-+p59rdFtV0Ug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 5, 2023 at 2:14 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> The rcancelrequested API is something that I devised out of whole cloth
> awhile ago. It's not in Tcl's copy of the code, which AFAIK is the
> only other project using this regex engine. I do still have vague
> hopes of someday seeing the engine as a standalone project, which is
> why I'd prefer to keep a bright line between the engine and Postgres.
> But there's no very strong reason to think that any hypothetical future
> external users who need a cancel API would want this API as opposed to
> one that requires exit() or longjmp() to get out of the engine. So if
> we're changing the way we use it, I think it's perfectly reasonable to
> redesign that API to make it simpler and less of an impedance mismatch.

Thanks for that background. Alright then, here's a new iteration
exploring this direction. It gets rid of CANCEL_REQUESTED() ->
REG_CANCEL and the associated error and callback function, and instead
has just "INTERRUPT(re);" at those cancellation points, which is a
macro that defaults to nothing (for Tcl's benefit). Our regcustom.h
defines it as CHECK_FOR_INTERRUPTS(). I dunno if it's worth passing
the "re" argument... I was imagining that someone who wants to free
memory explicitly and then longjmp would probably need it? (It might
even be possible to expand to something that sets an error and
returns, not investigated.) Better name or design very welcome.

Another decision is to use the no-OOM version of palloc. (Not
explored: could we use throwing palloc with attribute returns_nonnull
to teach GCC and Clang to prune the failure handling from generated
regex code?) (As for STACK_TOO_DEEP(): why follow a function pointer,
when it could be macro-only too? But that's getting off track.)

I split the patch in two: memory and interrupts. I also found a place
in contrib/pg_trgm that did no-longer-needed try/finally.

Attachment Content-Type Size
v5-0001-Use-MemoryContext-API-for-regex-memory-management.patch text/x-patch 6.0 KB
v5-0002-Update-contrib-trgm_regexp-s-memory-management.patch text/x-patch 1.5 KB
v5-0003-Redesign-interrupt-cancel-API-for-regex-engine.patch text/x-patch 12.1 KB
v5-0004-Fix-recovery-conflict-SIGUSR1-handling.patch text/x-patch 18.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-01-14 02:27:24 Re: How to find the number of cached pages for a relation?
Previous Message Andres Freund 2023-01-14 02:09:50 Re: Decoupling antiwraparound autovacuum from special rules around auto cancellation