From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
cc: pgsql-hackers <pgsql-hackers@postgresql.org>,
        Andres Freund <andres@anarazel.de>
Subject: Re: plpython tests fail against python 3.14 under Valgrind
In-reply-to: <2322557.1786409782@sss.pgh.pa.us>
References: <f071e691-5930-4738-9dbd-43ed2da367fe@gmail.com>
 <2322557.1786409782@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 10 Aug 2026 20:56:22 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2541684.1786481822.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 11 Aug 2026 16:57:02 -0400
Message-ID: <2541685.1786481822@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2541685.1786481822%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> Anyway, I've confirmed that the attached patch makes the problem
> go away here.  Unless somebody wants to expend brain cells on
> running the underlying issue to ground, I think we should just
> apply this to the back branches and be happy.  A variant plan
> could be to do it like this #ifdef USE_VALGRIND and otherwise
> keep the old code, but that seems unduly paranoid to me.

A bit of excavation in our git history reminded me that our old
behavior here was very intentional:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL9_6_BR [62c8421e8] 2016-07-07 11:28:17 -0400

    Reduce stack space consumption in tzload().
    =

    While syncing our timezone code with IANA's updates in commit 1c1a7cbd=
6,
    I'd chosen not to adopt the code they conditionally compile under #ifd=
ef
    ALL_STATE.  The main thing that that drives is that the space for gmti=
me
    and localtime timezone definitions isn't statically allocated, but is
    malloc'd on first use.  I reasoned we didn't need that logic: we don't=
 have
    localtime() at all, and we always initialize TimeZone to GMT so we alw=
ays
    need that one.  But there is one other thing ALL_STATE does, which is =
to
    make tzload() malloc its transient workspace instead of just declaring=
 it
    as a local variable.  It turns out that that local variable occupies 7=
8K.
    Even worse is that, at least for common US timezone settings, there's =
a
    recursive call to parse the "posixrules" zone name, making peak stack
    consumption to select a time zone upwards of 150K.  That's an uncomfor=
tably
    large fraction of our STACK_DEPTH_SLOP safety margin, and could result=
 in
    outright crashes if we try to reduce STACK_DEPTH_SLOP as has been disc=
ussed
    recently.  Furthermore, this means that the postmaster's peak stack
    consumption is several times that of a backend running typical queries
    (since, except on Windows, backends inherit the timezone GUC values an=
d
    don't ever run this code themselves unless you do SET TIMEZONE).  That=
's
    completely backwards from a safety perspective.
    =

    Hence, adopt the ALL_STATE rather than non-ALL_STATE variant of tzload=
(),
    while not changing the other code aspects that symbol controls.  The
    risk of an ENOMEM error from malloc() seems less than that of a SIGSEG=
V
    from stack overrun.

Now, we deleted the "posixrules" recursion logic in v13, so in all
active branches the amount of stack space required is 70-something K
not twice that.  But re-reading this message, the postmaster
vs. backends point still seems like a compelling safety argument.
So I'd rather make master work like the old code instead of vice
versa.

Hence, what I'm now thinking is to malloc the space by default,
but use a stack allocation if USE_VALGRIND, with a comment explaining
that that masks an obscure valgrind-or-possibly-python bug; and to
do it the same way in all branches.

			regards, tom lane