Re: BUG #19026: ResourceOwnerForget can't find owner for invalid plancache

On Wed, Aug 20, 2025 at 5:13 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
>
> Hi!
>
>
> First of all, Alexander, thank you for the excellent bug report!
>
> On Wed, 20 Aug 2025 at 16:21, Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > > ERROR:  XX000: plancache reference 0x643ae41a7d98 is not owned by resource
> > > owner Portal
> > > CONTEXT:  SQL function "part_hashint4_noop" during startup
> > > LOCATION:  ResourceOwnerForget, resowner.c:618
> >
> > The problem is in init_execution_state() we store the
> >
> > On error resource owner will be released, but that references are
> > still maintained in the plan and cowner in fcache, but on error the
> > transaction will be aborted and this resource owner will be released.
> > So next time when we try to clean up, it will access invalid memory.
> > So a simple fix would be to cleanup on error as attached POC.
> >
> > --
> > Regards,
> > Dilip Kumar
> > Google
>
> As for the patch: should we add some regression test for this?
> Also, I'm not terribly sure  what this fix does is the right thing to do.
> Doesn’t it break some layer of abstraction here? My understanding is
> that on transaction rollback, all resources should be freed in the
> ResourceOwnerRelease
> function and friends. Namely, the ReleaseCachedPlan call we make
> before elog(ERROR) is breaking logic. Am I wrong?
> For this sql cache there exists sql_exec_error_callback, which is
> probably a better place to clean up on error.
>
> How about attached?

Thanks, right it's better to clean up in sql_exec_error_callback.  But
we should still release the cached plan before setting it to NULL in
order to leak the cache plan as I was doing in my patch.  So I have
merged the logic of both the ideas and came up with this.

I will add a test for this and send it in sometime.

--
Regards,
Dilip Kumar
Google