pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedplpython tests fail against python 3.14 under Valgrind
3+ messages / 2 participants
[nested] [flat]
* plpython tests fail against python 3.14 under Valgrind
@ 2026-07-28 05:00 Alexander Lakhin <exclusion@gmail.com>
2026-08-11 00:56 ` Re: plpython tests fail against python 3.14 under Valgrind Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Lakhin @ 2026-07-28 05:00 UTC (permalink / raw)
To: pgsql-hackers
Hello hackers,
As buildfarm animal skink shows, upgrade to Python 3.14 breaks plpython
tests under Valgrind [1]:
61/400 plpython - postgresql:plpython/regress ERROR 49.88s exit status 1
pgsql.build/testrun/plpython/regress/log/postmaster.log
Fatal Python error: pycore_init_builtins: failed to add exceptions to builtins
Python runtime state: preinitialized
object address : 0x139da4a0
object refcount : 1
object type : 0x1388d020
object type name: RecursionError
object repr :
lost sys.stderr
Current thread 0x0000000008c62ec0 [memcheck-amd64-] (most recent call first):
<no Python frame>
I've reproduced this locally and confirmed that it's not reproduced with
PLPYTHON="python3.13" ./configure ...
It's also not reproduced on master, probably just because of a bit lesser
stack usage. I've bisected the anomaly and found what makes master pass
the tests: aeb07c55f.
AI suggests the offending change in Python 3.14 is [2]:
Use Py_EnterRecursiveCall() to guard against runaway recursion in C code
[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2026-07-27%2013%3A09%3A55
[2] https://docs.python.org/3/whatsnew/3.14.html
Best regards,
Alexander
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: plpython tests fail against python 3.14 under Valgrind
2026-07-28 05:00 plpython tests fail against python 3.14 under Valgrind Alexander Lakhin <exclusion@gmail.com>
@ 2026-08-11 00:56 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-08-11 20:57 ` Re: plpython tests fail against python 3.14 under Valgrind Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2026-08-11 00:56 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-hackers; Andres Freund <andres@anarazel.de>
Alexander Lakhin <exclusion@gmail.com> writes:
> As buildfarm animal skink shows, upgrade to Python 3.14 breaks plpython
> tests under Valgrind [1]:
> 61/400 plpython - postgresql:plpython/regress ERROR 49.88s exit status 1
Yeah, I can reproduce this here, using Fedora 43 (with
python3-3.14.6-1.fc43.x86_64 and valgrind-3.27.1-1.fc43.x86_64).
As you say, it doesn't reproduce on master; I didn't take the trouble
to bisect.
> It's also not reproduced on master, probably just because of a bit lesser
> stack usage. I've bisected the anomaly and found what makes master pass
> the tests: aeb07c55f.
Actually, I think it's the other way around: master uses more stack.
In v19, tzload() does this:
union local_storage *lsp = malloc(sizeof *lsp);
...
free(lsp);
while in master it does this:
union local_storage *lsp;
union local_storage ls;
lsp = &ls;
That's a sizeable amount of stack getting chewed:
(gdb) p sizeof(union local_storage)
$1 = 72240
However, we surely don't invoke tzload() while running any Python
code. What I think must be happening is that at postmaster start,
or possibly backend start, we invoke tzload() while setting the
timezone GUC, and this causes a bunch of stack pages to get allocated,
more than we have in the v19 code path. Somehow, python and valgrind
interact badly when there's not much pre-existing stack allocation.
It's pretty unclear which one is to blame, but it seems like it can't
be our fault. (Although ... surely the standard python executable
invokes libpython with little pre-existing stack? If that doesn't
trigger this problem, what are we doing differently?)
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.
regards, tom lane
Attachments:
[text/x-diff] tzload-stack-hack-for-python.patch (674B, ../../2322557.1786409782@sss.pgh.pa.us/2-tzload-stack-hack-for-python.patch)
download | inline diff:
diff --git a/src/timezone/localtime.c b/src/timezone/localtime.c
index fb04b4cf6bf..1a8088b520c 100644
--- a/src/timezone/localtime.c
+++ b/src/timezone/localtime.c
@@ -588,17 +588,10 @@ tzloadbody(char const *name, char *canonname, struct state *sp, bool doextend,
int
tzload(char const *name, char *canonname, struct state *sp, bool doextend)
{
- union local_storage *lsp = malloc(sizeof *lsp);
+ union local_storage ls;
+ union local_storage *lsp = &ls;
- if (!lsp)
- return errno;
- else
- {
- int err = tzloadbody(name, canonname, sp, doextend, lsp);
-
- free(lsp);
- return err;
- }
+ return tzloadbody(name, canonname, sp, doextend, lsp);
}
static bool
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: plpython tests fail against python 3.14 under Valgrind
2026-07-28 05:00 plpython tests fail against python 3.14 under Valgrind Alexander Lakhin <exclusion@gmail.com>
2026-08-11 00:56 ` Re: plpython tests fail against python 3.14 under Valgrind Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-08-11 20:57 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2026-08-11 20:57 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-hackers; Andres Freund <andres@anarazel.de>
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 1c1a7cbd6,
I'd chosen not to adopt the code they conditionally compile under #ifdef
ALL_STATE. The main thing that that drives is that the space for gmtime
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 always
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 78K.
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 uncomfortably
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 discussed
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 and
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 SIGSEGV
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
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-08-11 20:57 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-07-28 05:00 plpython tests fail against python 3.14 under Valgrind Alexander Lakhin <exclusion@gmail.com>
2026-08-11 00:56 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-08-11 20:57 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox