On Sat, Dec 31, 2022 at 6:36 PM Noah Misch <noah@leadboat.com> wrote:
> On Sat, Dec 31, 2022 at 10:06:53AM +1300, Thomas Munro wrote:
> > Idea #8 is a realisation that twisting oneself into a pretzel to avoid
> > having to change the regexp code or its REG_CANCEL control flow may be
> > a bit silly. If the only thing it really needs to do is free some
> > memory, maybe the regexp module should provide a function that frees
> > everything that is safe to call from our rcancelrequested callback, so
> > we can do so before we longjmp back to Kansas. Then the REG_CANCEL
> > code paths would be effectively unreachable in PostgreSQL. I don't
> > know if it's better or worse than your idea #6, "use palloc instead,
> > it already has garbage collection, duh", but it's a different take on
> > the same realisation that this is just about free().
>
> PG_TRY() isn't free, so it's nice that (6) doesn't add one. If (6) fails in
> some not-yet-revealed way, (8) could get more relevant.
>
> > I guess idea #6 must be pretty easy to try: just point that MALLOC()
> > macro to palloc(), and do a plain old CFI() in rcancelrequested().
It's not quite so easy: in RE_compile_and_cache we construct objects
with arbitrary cache-managed lifetime, which suggests we need a cache
memory context, but we could also fail mid construction, which
suggests we'd need a dedicated per-regex object memory context that is
made permanent with the MemoryContextSetParent() trick (as we see
elsewhere for cached things that are constructed by code that might
throw), or something like the try/catch thing from idea #8.
Thinking...