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

On Tue, Apr 4, 2023 at 1:25 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Sorry for not looking at this sooner. I am okay with the regex
> changes proposed in v5-0001 through 0003, but I think you need to
> take another mopup pass there. Some specific complaints:
> * header comment for pg_regprefix has been falsified (s/malloc/palloc/)

Thanks. Fixed.

> * in spell.c, regex_affix_deletion_callback could be got rid of

Done in a separate patch. I wondered if regex_t should be included
directly as a member of that union inside AFFIX, but decided it should
keep using a pointer (just without the extra wrapper struct). A
direct member would make the AFFIX slightly larger, and it would
require us to assume that regex_t is movable which it probably
actually is in practice I guess but that isn't written down anywhere
and it seemed strange to rely on it.

> * check other callers of pg_regerror for now-useless CHECK_FOR_INTERRUPTS

I found three of these to remove (jsonpath_gram.y, varlena.c, test_regex.c).

> In general there's a lot of comments referring to regexes being malloc'd.

There is also some remaining direct use of malloc() in
regc_pg_locale.c because "we mustn't lose control on out-of-memory".
At that time (2012) there was no MCXT_NO_OOM (2015), so we could
presumably bring that cache into an observable MemoryContext now too.
I haven't written a patch for that, though, because it's not in the
way of my recovery conflict mission.

> I'm disinclined to change the ones inside the engine, because as far as
> it knows it is still using malloc, but maybe we should work harder on
> our own comments. In particular, it'd likely be useful to have something
> somewhere pointing out that pg_regfree is only needed when you can't
> get rid of the regex by context cleanup. Maybe write a short section
> about memory management in backend/regex/README?

I'll try to write something for the README tomorrow. Here's a new
version of the code changes.

> I've not really looked at 0004.

I'm hoping to get just the regex changes in ASAP, and then take a
little bit longer on the recovery conflict patch itself (v6-0005) on
the basis that it's bugfix work and not subject to the feature freeze.