Re: PL/Python: Fix return in the middle of PG_TRY() block.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Xing Guo <higuoxing(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PL/Python: Fix return in the middle of PG_TRY() block.
Date: 2023-05-03 20:33:32
Message-ID: 3012766.1683146012@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Nathan Bossart <nathandbossart(at)gmail(dot)com> writes:
> Here's a new version of the patch. Besides adding comments and a commit
> message, I made sure to decrement the reference count for pltargs in the
> PG_CATCH block (which means that pltargs likely needs to be volatile).

Hmm, actually I think these changes should allow you to *remove* some
volatile markers. IIUC, you need volatile for variables that are declared
outside PG_TRY but modified within it. That is the case for these
pointers as the code stands, but your patch is changing them to the
non-risky case where they are assigned once before entering PG_TRY.

(My mental model of this is that without "volatile", the compiler
may keep the variable in a register, creating the hazard that longjmp
will revert the variable's value to what it was at setjmp time thanks
to the register save/restore that those functions do. But if it hasn't
changed value since entering PG_TRY, then that doesn't matter.)

> I'm
> not too wild about moving the chunk of code for pltargs like this, but I
> haven't thought of a better option. We could error instead of returning
> NULL, but IIUC that would go against d0aa965's stated purpose.

Agreed, throwing an error in these situations doesn't improve matters.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-05-03 20:58:38 Re: PL/Python: Fix return in the middle of PG_TRY() block.
Previous Message Nathan Bossart 2023-05-03 20:21:16 Re: PL/Python: Fix return in the middle of PG_TRY() block.