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

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> I think this experiment worked out pretty well. I think it's a nice
> side-effect that you can see what memory the regexp subsystem is
> using, and that's likely to lead to more improvements. (Why is it
> limited to caching 32 entries? Why is it a linear search, not a hash
> table? Why is LRU implemented with memmove() and not a list? Could
> we have a GUC regex_cache_memory, so someone who uses a lot of regexes
> can opt into a large cache?) On the other hand it also uses a bit
> more RAM, like other code using the reparenting trick, which is a
> topic for future research.

> I vote for proceeding with this approach. I wish we didn't have to
> tackle either a regexp interface/management change (done here) or a
> CFI() redesign (not done, but probably also a good idea for other
> reasons) before getting this signal stuff straightened out, but here
> we are. This approach seems good to me. Anyone have a different
> take?

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/)
* in spell.c, regex_affix_deletion_callback could be got rid of
* check other callers of pg_regerror for now-useless CHECK_FOR_INTERRUPTS

In general there's a lot of comments referring to regexes being malloc'd.
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've not really looked at 0004.

regards, tom lane