NilPy still leaks class fields under --threadsafe
- Track A (heap lock discipline). Measured 2026-08-31 by frankS
immediately after fixing the Pascal half
(
bug-a-threadsafe-on-x86-64-leaks-every-managed-class-field-and-it-is-not-benign). - Filed rather than fixed because it is the same SYMPTOM through a different MECHANISM, and the tempting move is to assume otherwise.
The number
class Holder:
def __init__(self, s):
self.s = s
# 200000 x Holder("y"*2000 + str(i))
| build | max RSS |
|---|---|
pascal26 npyleak.npy |
1044 kB |
pascal26 --threadsafe npyleak.npy |
399420 kB |
Same printed answer, 380x the memory. Compare the Pascal probe in the parent ticket: 392 kB -> 398336 kB. Same shape, same size, and it survived the fix.
Why it did not ride along
The Pascal fix works because a Pascal class's finalize has exactly one
reachable call site — the Free desugar in ir.inc — so the acquire could be
emitted THERE, around a call to the new PXXClassFinalizeManaged, and the
callee needs no lock of its own.
NilPy has no such site. ir.inc deliberately does not emit
PXXClassFinalize for a NilPy compilation (if not isNilPy, and the comment
above it records why: emitting it there finalized a headered instance twice).
The finalize instead runs from PXXObjRelease when a refcount reaches zero, via
PXXObjFinalizeHook -> PyObjFinalize -> PXXClassFinalize, whose managed half
is still behind {$ifndef PXX_TS_HARDLOCK}. PXXObjRelease is reached from the
object retain/release blobs (EmitObjBlobBody, which takes no lock), from
PXXRecordRelease's kind-6 arm, from PXXVarClear, and from Pascal inside the
runtime. There is no single place to put an acquire.
The two things that make it a design problem, not a patch
- The release blobs take no lock at all today.
EmitObjBlobBody(ir_codegen.inc) is a bare register-save wrapper around the Pascal proc. Wrapping it is the obvious move and is not obviously safe: the callee runs a user finalizer (container teardown, and in principle any NilPy-level__del__-shaped work), which is the same reasonPXXClassFinalize's kind-4 pass must stay OUTSIDE the lock. - The lock is not reentrant. Anything reached under it that performs a
string concat, a literal load or a
SetLengthgoes through a blob that acquires again, and the program HANGS rather than crashing. The Pascal fix is safe only because its subtree is provably runtime-only;test_threadsafe_class_finalize_kinds.pasexists to keep that true. A NilPy finalizer has no such property.
What is NOT known and should be measured first — ANSWERED 2026-09-02 (frankA)
Can a NilPy program create a thread at all today? (the original question, kept because the answer only means something against it:) If it cannot, every one of these frees is single-threaded and the gate is buying nothing on this path — in which case the fix is to let the NilPy route run the managed pass unconditionally and the whole design problem above evaporates. If it can, none of that holds.
It can. The design problem does not evaporate. pyparser.inc has a
__pxxclone(flags, childStack, entry, arg, ctidptr) builtin that refuses to
compile without --threadsafe, and a NilPy program using it starts a real
thread that runs real NilPy code: constructed, run, and now wired as
test/test_nilpy_thread_clone.npy — mmap a stack, clone, and the child sets a
global the parent spins on. 5 runs of 5, child ran = 7.
Nobody had ever constructed one, and the natural spelling was broken, which
is presumably why this stayed unmeasured: no .npy in the tree used
__pxxclone. Passing the entry point as a bare def name — the only spelling
NilPy has, since it has no @ — got the BOXED callable every other value
position gets, and the box may even be a synthesized return-side wrapper with a
different ABI. So the child jumped into a value handle: on the pin, this exact
source is rc=139 three runs of three, with tid nonzero = True already printed.
Fixed in the same push (a bare def name at that one argument is read as an
address, as Pascal reads @ThreadEntry at the same position). The measurement
above is against the FIXED compiler; against the pin the answer is "it creates
the thread and the thread dies instantly", which is a yes to the question this
section asks and a no to any use of it.
So the frees on this path can genuinely race, PXX_TS_HARDLOCK is not gating
nothing, and the two obstacles below stand as written.
Also still open, same lock, named in the parent
RECORD COM-interface fields (PXXRecordReleaseIntf) are the same
benign-by-assertion leak under the same lock, and were never measured either.
2026-09-02 (frankA) — the design, the one thing that blocks it, and why the experiment is now cheap
Leak reproduced at HEAD, same program, so the numbers below are against a live
defect and not a historical one: 1024 kB plain vs 399524 kB --threadsafe,
390x, against the filed 1044 / 399420.
"There is no single place to put an acquire" is not the constraint
The ticket's second obstacle rests on it, and the mechanism does not work the
way it assumes. HeapLockedCallProcIdx1 keys on the CALLEE, not on a call
site: ir_codegen.inc's IR_CALL arm tests if procIdx + 1 = HeapLockedCallProcIdx1 and wraps the call in EmitAcquireHeapLock /
EmitReleaseHeapLock, evaluating the argument outside the lock. So every
IR_CALL to PXXClassFinalizeManaged gets the lock, wherever it is written —
including one written in Pascal inside the runtime, such as the line at
builtinheap.pas:4245 that {$ifndef PXX_TS_HARDLOCK} currently compiles out.
A NilPy compilation does not need a place to put an acquire. It needs that
{$ifndef} removed and the global set.
Two things that then have to be settled, both concrete
-
The global is only ever set by Pascal-only lowerings.
ir.inc:11946(theFreedesugar) andir.inc:14187(the caught-exception owner free) are the only writers, and a NilPy compilation lowers neither, soHeapLockedCallProcIdx1stays 0 and the call would be emitted UNLOCKED — which is a data race rather than a leak, i.e. a different bug, not a fix. It wants one assignment per compilation as soon as the proc row exists, with the two existing sites delegating to it rather than a third copy (normalise-dont-special-case). -
Removing the gate double-releases on the Pascal path, because the
Freedesugar then emits the managed sweep a second time. The unification — drop the second emitted call, letPXXClassFinalizemake the only one, and let the callee-keyed wrapper supply the lock — deletes a path instead of adding one, and is the shape to aim at.
And the reason it is not free: the recursion is NilPy-specific
PXXClassFinalizeManaged <- lock acquired here by the wrapper
PXXRecordRelease
kind 6 (a NilPy-object field)
PXXObjRelease -> rc = 0
PXXObjFinalizeHook -> PyObjFinalize
PXXClassFinalize
PXXClassFinalizeManaged <- the wrapper acquires AGAIN
The second acquire waits on a lock the first still holds, on the same thread. This is obstacle 2 of this ticket, and it is now a specific chain rather than a worry — and it is why the Pascal path this design came from cannot see it: a Pascal class field holding another class instance is not ARC-managed, so kind 6 never appears there. Unification looks free from the Pascal side precisely because the Pascal side cannot reach the case that breaks it.
What changed since this was filed, and it changes the cost of finding out
"the program HANGS rather than crashing" was the expensive part — a hang is
indistinguishable from slow, so every experiment cost a timeout and returned
nothing. 187a372a6 made that hang a named diagnosis: the contended heap
lock now writes Runtime error 212: the heap lock was never released and
exits 212, measured 1.9s from the collision, 6 runs of 6.
So the experiment this ticket has been waiting for is now one build and one run:
remove the {$ifndef PXX_TS_HARDLOCK} at builtinheap.pas:4245, drop the
Free desugar's second emitted call, set HeapLockedCallProcIdx1 once per
compilation, and run a NilPy program with a class whose field is another class
instance. Either the leak goes to zero and nothing deadlocks — in which case
the recursion above is not reachable for the shapes that matter and wants a test
proving so — or it exits 212 in two seconds naming the exact chain, which makes
the reentrant heap lock (feature-a-reentrant-heap-lock-and-per-thread-arenas,
and option 2 of
[[bug-a-the-threadsafe-allocator-is-not-async-signal-safe]]) the blocker, wired
as a hard blocked-by. RUN — see the next section. (The sentence that stood
here, "Not run here: it changes the shared Pascal path and belongs in a session
that can carry the full-tier verification that implies", was a false limit.
PXX_ALLOW_FULL_SUITE=1 is a SPEED guardrail, not a permission gate — CLAUDE.md
says so in as many words — so the session that wrote that sentence could have run
it, and did, an hour later.)
2026-09-02 (frankA) — the experiment RAN, and it returned the second branch
Compiler cea696760e57 at 94075d508 for the two control arms, and
1d8aa6ed139b for the unification arm — the same tree with exactly the three
edits the section above prescribes, applied, built (converged after 1 round(s)
both ways), measured, and reverted back to a byte-identical cea696760e57.
Two NilPy programs, 200000 constructions each, /usr/bin/time -f %M, magazine
on (the default), all four arms printing the same correct 401088890:
| program | plain | --threadsafe (HEAD) |
--threadsafe + unification |
|---|---|---|---|
| field is a string | 7672 kB | 399524 kB | 7844 kB |
| field is a class instance | 7672 kB | 410276 kB | Runtime error 212, rc=212 |
The unification fixes the leak and converts the nested case into a deadlock. Both halves of the prediction, on the two halves of the input.
A deadlock is worse than a leak, so it is reverted rather than landed. Nothing
here is a reason to doubt the design: the three Pascal rows the unification also
changes stayed green under it, 3 runs of 3 each —
test_threadsafe_class_finalize_race (errors=0 / RACE OK),
test_threadsafe_class_finalize_kinds (errors=0 / KINDS OK), and
test_threadsafe_exception_managed_fields (caught=3000,
allocs=10975 frees=10972 live=3, bound 200). The deleted second emission and
the callee-keyed stamp do what they were supposed to do.
The repro is twelve lines, not two hundred thousand
class Inner:
def __init__(self, s):
self.s = s
class Outer:
def __init__(self):
self.f = Inner("hello")
o = Outer()
o = 0
print("done")
One instance, released once. This is not a contention bug and no second thread is involved; it is a single thread taking a non-reentrant lock twice.
Which field kinds actually trigger it — the narrowing
Same program, five spellings of self.f, --threadsafe + unification:
self.f = |
result |
|---|---|
42 |
done |
"hello" |
done |
[1, 2, 3] |
done |
{"a": 1} |
done |
Inner("hello") |
rc=212 |
So it is not "any ARC-managed NilPy object field". A list and a dict field
are kind 6 too and both release cleanly: their teardown does not route through
PyObjFinalize. The trigger is specifically a field holding a user class
instance, i.e. the one value whose release re-enters PXXClassFinalize and
therefore calls PXXClassFinalizeManaged — the wrapper's own callee — a second
time on the same thread. The chain named in the section above is right, and the
measurement narrows its middle to one arm.
And the framing that made this NilPy-only holds by measurement, not by argument:
the same shape in Pascal — TOuter with a TInner field and an AnsiString,
both Freed — runs done, rc=0, under the unification. A Pascal class field
holding a class is not ARC-managed, so kind 6 never appears and the Pascal path
structurally cannot reach the case that breaks.
What this leaves, and why it is not a blocked-by on the obvious ticket
feature-a-reentrant-heap-lock-and-per-thread-arenas is in done/ — for the
ALLOCATOR half. Its own summary says the reentrancy half "stays parked by the
owner and was never touched", so wiring a hard blocked-by: at it would point
this ticket at a closed ticket and read as satisfied. The blocker is the parked
HALF, and unparking it is the owner's call.
There is also a second arm that is not reentrancy at all: defer the nested
release. The kind-6 arm pushes the object onto a per-thread pending list
instead of calling PXXObjRelease, and the codegen wrapper drains it right
after EmitReleaseHeapLock — the one place that already knows the lock is being
dropped. It needs no change to the lock primitive, which is the part the owner
parked, and it costs one TLS slot. It is not free either: it moves a finalizer's
execution to after the outer walk, which is an observable ordering change for
anything with a user-visible __del__.
Two arms, one parked by the owner and one with a semantic cost — that is a fork
of intent rather than a defaulted decision, so it is filed as
[[decide-a-how-should-the-nilpy-managed-finalize-re-enter-the-heap-lock]] and
this ticket is blocked-by that.
2026-09-06 — FIXED. The reverted change came back, on a lock that re-enters.
The owner ruled arm (a) of the fork this was blocked on, the heap lock gained
owner+depth (feature-a-make-the-heap-lock-reentrant), and this ticket's own
already-built fix landed essentially as described:
{$ifndef PXX_TS_HARDLOCK}removed frombuiltinheap.pas'sPXXClassFinalizeManaged(inst)call, soPXXClassFinalizefinalizes its own managed fields again, for every route to it and not only for theFreedesugar;HeapLockedCallProcIdx1stamped once per compilation inEmitHeapLockStubs, which is the load-bearing half — it keys on the CALLEE, so one stamp wraps every call toPXXClassFinalizeManagedin the lock, including the onePXXClassFinalizenow makes itself. Stamped lazily by the desugar, a program that never desugars aFreenever stamped it and the walk ran unlocked;- both duplicate second emissions deleted — the
Freedesugar's, and the caught-exception one added earlier the same day — because a second call is now a DOUBLE finalize, i.e. a double free rather than a leak.
200000 NilPy constructions under --threadsafe, maxrss:
| pinned (pre-change) | 19760 kB |
| HEAD | 1048 kB |
A correction to this ticket's own account, worth having
The twelve-line repro does not deadlock at HEAD, and that is measured rather
than inferred. With the {$ifndef} removed and the stamp in place, it runs rc=0
with reentrancy switched OFF (-dPXX_NO_REENTRANT_HEAPLOCK) as well as on, and a
deeper NilPy shape — nested class fields plus a list and a dict — does the same.
So the recursion this ticket describes is not reachable by that program on
today's tree, and the leak fix above did not need the reentrant lock.
Where the reentrancy IS load-bearing, demonstrated with a control that
discriminates, is the sibling row:
bug-a-threadsafe-builds-leak-every-variant-and-interface-element-of-a-dynamic-array.
Lifting ManagedElemKindLocked's degradation with reentrancy off makes that
program hit rc=212 with the heap-lock diagnosis; with it on, 7939 live becomes 3.
That does not make the fork's decision wrong — it makes this ticket's repro the wrong instrument for it. Recorded because the next reader would otherwise take "twelve lines, one thread, exit 212" as still reproducible and spend an hour on a program that no longer does it.
Inert until pinned
This is a compiler-side change with a lib/** consumer (builtinheap.pas is a
compiler build input), so $(PXX_STABLE) consumers do not get it until the next
pin. Not waiting for one — landing forward and saying so, per the pin rules.
- 2026-09-06 — resolved, commit 3bb71fd79 (the fix and the close are one commit).