public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19480: PL/Python SRF crashes (SIGSEGV) when function is replaced mid-iteration: use-after-free in PLy_funct
Date: Wed, 17 Jun 2026 17:56:39 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

"Matheus Alcantara" <[email protected]> writes:
> Patch attached.

I had been planning to wait for v20 development to open, but with
RMT approval the target is now v19 instead, so I'd like to get
this done before the end of June.  I looked through the patch
and found a couple of issues immediately:

* Your refactoring to have just one PLy_procedure_get call in
plpython3_call_handler is no good.  You missed the comment
block just above:

    /*
     * Push execution context onto stack.  It is important that this get
     * popped again, so avoid putting anything that could throw error between
     * here and the PG_TRY.
     */
    exec_ctx = PLy_push_execution_context(!nonatomic);

+   proc = PLy_procedure_get(fcinfo, false);
+
    PG_TRY();
    {

I counsel putting those PLy_procedure_get calls back where they were.

* I also question the decision to refactor where/how is_trigger is
computed; that doesn't seem necessary to the purposes of the patch,
nor is it a clear improvement.  I'd just as soon leave that
mechanism alone as much as we can.  If there is an improvement to
be had, let's address that separately.  (Alternative thought:
should we rely on the isTrigger/isEventTrigger bools that
funccache.c sets up for us?  I'm not quite sure if getting friendly
with struct CachedFunctionHashKey is a good idea or not.)

* I find it confusing that you called "PLyProcedureCache *" variables
"pcache" in some places and "proc" in others.  The latter choice seems
poor because mostly "proc" is a PLyProcedure pointer.  Using "proc"
leads to constructions like "proc->proc", which I don't find
intelligible.

* The new code could do with more comments.  I realize that plpython
is poorly commented in many places, but let's see if we can leave it
better than we found it.

			regards, tom lane

view thread (17+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #19480: PL/Python SRF crashes (SIGSEGV) when function is replaced mid-iteration: use-after-free in PLy_funct
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox