From 00e751415c6262b8f47e5237566a37fdfad92dae Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sun, 28 Jun 2026 17:56:20 +0100 Subject: [PATCH 1/2] refactor(engine): migrate per-thread cache clears into the cleanup registry (#893) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #885/#891. PR #891 added Goccia.ThreadCleanupRegistry for the ~64 member-definition threadvars but left seven per-thread cache/memo clears as explicit calls inside Goccia.Threading.ShutdownThreadRuntime, leaving two cleanup idioms coexisting on the worker-exit path and coupling Goccia.Threading to seven cache-owning units. Each unit now registers its clear with the registry from its own initialization (matching the established pattern), and ShutdownThreadRuntime drops the explicit calls — keeping only the RunThreadvarCleanups drain before the ordered GC/CallStack/MicrotaskQueue shutdowns. Goccia.Threading no longer uses any of the seven units. - Atomics: register the per-thread ShutdownAtomicsWaitersForCurrentThread; keep the all-threads ShutdownAtomicsWaiters + lock teardown in the unit finalization (per-thread/all-threads distinction preserved); guard the per-thread proc against the registry's post-teardown main-thread drain. - TextSemantics (generic shared infra): register its is-ASCII memo clear from Goccia.RegExp.VM to keep the shared unit engine-free, and retain its own main-thread finalization for binaries that link it without the regex VM. - Broaden the registry callback contract to cover a thread's own entries in a shared lock-guarded structure (Atomics), not only managed threadvars. - Add a Goccia.ThreadCleanupLeak.Test gate pinning each of the seven registrations, plus ADR 0080. Co-Authored-By: Claude Opus 4.8 --- ...rate-cache-clears-into-cleanup-registry.md | 28 +++++++++++++++++ docs/adr/README.md | 1 + source/shared/TextSemantics.pas | 21 ++++++++++--- source/units/Goccia.Builtins.Atomics.pas | 23 ++++++++++++++ .../units/Goccia.Builtins.DisposableStack.pas | 8 +++-- source/units/Goccia.Builtins.Semver.pas | 8 +++-- source/units/Goccia.ImportMeta.pas | 13 +++++--- source/units/Goccia.RegExp.VM.pas | 28 ++++++++++++----- source/units/Goccia.Temporal.TimeZone.pas | 12 ++++--- .../units/Goccia.ThreadCleanupLeak.Test.pas | 30 ++++++++++++++++++ source/units/Goccia.ThreadCleanupRegistry.pas | 29 +++++++++++++++-- source/units/Goccia.Threading.pas | 31 +++++++------------ 12 files changed, 183 insertions(+), 49 deletions(-) create mode 100644 docs/adr/0080-migrate-cache-clears-into-cleanup-registry.md diff --git a/docs/adr/0080-migrate-cache-clears-into-cleanup-registry.md b/docs/adr/0080-migrate-cache-clears-into-cleanup-registry.md new file mode 100644 index 000000000..8495c71f8 --- /dev/null +++ b/docs/adr/0080-migrate-cache-clears-into-cleanup-registry.md @@ -0,0 +1,28 @@ +# Migrate explicit per-thread cache clears into the thread-cleanup registry + +**Date:** 2026-06-28 +**Area:** `engine` +**Issue:** [#893](https://github.com/frostney/GocciaScript/issues/893) + +[ADR 0078](0078-thread-local-cleanup-registry.md) added `Goccia.ThreadCleanupRegistry` and routed the ~64 member-definition threadvars through it, but to avoid destabilising working teardown in [#891](https://github.com/frostney/GocciaScript/pull/891) it left seven pre-existing per-thread cache/memo clears as **explicit calls** inside `Goccia.Threading.ShutdownThreadRuntime`: `ClearImportMetaCache`, `ShutdownAtomicsWaitersForCurrentThread`, `ClearDisposableStackSlotMap`, `ClearSemverHosts`, `ClearTimeZoneCache`, `ClearRegExpInputMemo`, and `ClearAsciiMemo`. That left two cleanup idioms coexisting on the worker-exit path — explicit calls plus the registry drain — and coupled `Goccia.Threading`'s `uses` clause to seven cache-owning units. + +## Decision + +Each of the seven units registers its clear with `Goccia.ThreadCleanupRegistry` from its own `initialization` section, the same shape the 64 member-definition threadvars already use, and `ShutdownThreadRuntime` drops the seven explicit calls — keeping only the `RunThreadvarCleanups` drain followed by the ordered object-lifecycle shutdowns (MicrotaskQueue / CallStack / GarbageCollector), which stay explicit because their order matters. The drain still runs **before** those shutdowns, so a cache that touches the collector (`ClearImportMetaCache` unpins) is released while the GC is alive. `Goccia.Threading` no longer `uses` any of the seven units. + +Units whose `finalization` existed only to clear a threadvar drop it and rely on the registry's own finalization for main-thread cleanup (`ImportMeta`, `DisposableStack`, `Semver`, `RegExp.VM`). Units whose finalization does more keep that part: `Temporal.TimeZone` retains the Windows ICU-lock teardown, and `Atomics` retains its all-threads shutdown (below). + +Three cases needed care: + +- **Atomics** is not a managed threadvar. `GAtomicsWaiters` is a shared, lock-guarded global; `ShutdownAtomicsWaitersForCurrentThread` removes only the calling thread's entries (keyed by owner thread id). It is registered for the worker path, but the unit's own `finalization` keeps the all-threads `ShutdownAtomicsWaiters` plus `DoneCriticalSection` for the main thread — preserving the per-thread/all-threads distinction. Because the registry finalizes *after* this unit (it is an earlier-initialised dependency), the registry's main-thread drain would otherwise call the per-thread proc *after* the lock is destroyed; a pre-lock `if not Assigned(GAtomicsWaiters)` guard makes that a safe no-op. The registry's callback contract is broadened to cover "a thread's own entries in a shared lock-guarded structure," not only managed threadvars. + +- **TextSemantics** is generic shared infrastructure (`source/shared/`) with no engine dependency, used by the JSON, numeric-text and CLI-config tools as well as the engine. To keep it engine-free, its is-ASCII memo (#806) clear is registered from the engine's `Goccia.RegExp.VM` rather than self-registered — every engine binary links the regex VM, so the worker path is covered. Its **own main-thread `finalization` is retained**, because binaries that link `TextSemantics` but not `RegExp.VM` (the shared tools and `TextSemantics.Test`) populate the memo and would otherwise never release it. + +- **ImportMeta** keeps `ClearImportMetaCache` exported: besides the registry, the engine calls it directly during its own teardown. + +## Consequences + +- Worker-thread cleanup is one mechanism (the registry drain); `Goccia.Threading` is decoupled from the cache-owning units. Adding a new per-thread cache no longer means editing `ShutdownThreadRuntime`. +- Main-thread cleanup is preserved on whichever path is correct per unit: the registry finalization for the self-registering engine units, and the unit's own finalization for `Atomics` (all-threads), the Windows ICU lock, and `TextSemantics` (binary-independent). +- A new gate in `Goccia.ThreadCleanupLeak.Test` pins each of the seven registrations via `IsThreadvarCleanupRegistered`, so a dropped `RegisterThreadvarCleanup` fails loudly instead of silently leaking. `Goccia.Threading.Test` still proves the drain fires once per worker exit. The full JavaScript suite passes in both modes and all Pascal unit tests pass. +- No behaviour or conformance change: this is the cleanliness/embeddability follow-up ADR 0078 deferred, not a runtime change. diff --git a/docs/adr/README.md b/docs/adr/README.md index 5252f9477..3aea5132a 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -89,3 +89,4 @@ Durable architecture and implementation decisions for GocciaScript. New ADRs use - [0077 — SameValueZero-keyed ordered store for Map and Set](0077-samevaluezero-ordered-collections.md) - [0078 — Thread-local cleanup registry for managed threadvars](0078-thread-local-cleanup-registry.md) - [0079 — Keep speculatively-scanned tokens across parenthesized-group probes](0079-keep-speculatively-scanned-tokens.md) +- [0080 — Migrate explicit per-thread cache clears into the thread-cleanup registry](0080-migrate-cache-clears-into-cleanup-registry.md) diff --git a/source/shared/TextSemantics.pas b/source/shared/TextSemantics.pas index c677a570f..17ed3159e 100644 --- a/source/shared/TextSemantics.pas +++ b/source/shared/TextSemantics.pas @@ -71,9 +71,13 @@ function NormalizeNewlinesToLF(const AText: string): string; function NormalizeUTF8NewlinesToLF(const AText: UTF8String): UTF8String; function StringListToLFText(const ALines: TStrings): string; -{ Release the per-thread is-ASCII memo. Called from this unit's finalization - (main thread) and from ShutdownThreadRuntime (worker threads), because FPC - does not auto-finalize managed threadvars at thread exit. } +{ Release the per-thread is-ASCII memo. FPC does not auto-finalize managed + threadvars at thread exit. This unit's own finalization clears the main + thread's slots on process shutdown; because the unit stays free of engine + dependencies, that path works in every binary that links it (including the + shared JSON/numeric/config tools that never link the engine). Worker threads + are covered separately: the engine's Goccia.RegExp.VM registers this proc with + Goccia.ThreadCleanupRegistry, whose drain releases each worker's slots on exit. } procedure ClearAsciiMemo; implementation @@ -1262,8 +1266,10 @@ function StringListToLFText(const ALines: TStrings): string; Result := Buffer.ToString; end; -// Release the is-ASCII memo strings on shutdown; FPC does not finalize managed -// threadvars at thread exit (see the memo declaration above). +// Release the is-ASCII memo strings; FPC does not finalize managed threadvars at +// thread exit. Run from this unit's own finalization on the main thread, and — +// for worker threads — via Goccia.ThreadCleanupRegistry, where Goccia.RegExp.VM +// registers it (see the declaration comment above). procedure ClearAsciiMemo; begin GAsciiMemoStr0 := ''; @@ -1279,6 +1285,11 @@ initialization {$ENDIF} finalization + // Main-thread cleanup, kept here because this generic unit has no engine + // dependency and so cannot self-register with Goccia.ThreadCleanupRegistry; + // it runs in every binary that links TextSemantics, including ones that never + // link the engine (Goccia.RegExp.VM registers the worker-thread path). FPC + // does not auto-finalize managed threadvars at thread exit. ClearAsciiMemo; end. diff --git a/source/units/Goccia.Builtins.Atomics.pas b/source/units/Goccia.Builtins.Atomics.pas index c663ac5e0..64444adf1 100644 --- a/source/units/Goccia.Builtins.Atomics.pas +++ b/source/units/Goccia.Builtins.Atomics.pas @@ -75,6 +75,7 @@ implementation Goccia.Error.Messages, Goccia.GarbageCollector, Goccia.InstructionLimit, + Goccia.ThreadCleanupRegistry, Goccia.Timeout, Goccia.Values.ArrayBufferValue, Goccia.Values.BigIntValue, @@ -908,6 +909,22 @@ procedure ShutdownAtomicsWaitersForCurrentThread; Waiter: TAtomicsWaiter; Waiters: TObjectList; begin + // Registered with Goccia.ThreadCleanupRegistry, so the drain runs this on + // worker exit AND again at main-thread finalization — by which point this + // unit's own finalization has already run ShutdownAtomicsWaiters (nil-ing + // GAtomicsWaiters) and DoneCriticalSection'd GAtomicsLock. Bail out before + // touching the now-destroyed lock when the list is already gone. + // + // This pre-lock read of the shared GAtomicsWaiters is deliberately unlocked. + // On the main-thread finalization path the process is single-threaded, so it + // cannot race. On a worker-exit path the lock is alive and another worker may + // be creating GAtomicsWaiters (nil -> non-nil) under it, but the read still + // gives THIS thread a correct answer: a thread only has waiters to remove + // after the list was created (creation precedes any Add), so reading nil means + // this thread has nothing to clean. It is a single aligned-pointer load — + // atomic on supported targets, never torn. + if not Assigned(GAtomicsWaiters) then + Exit; RemovedWaiters := TList.Create; try CurrentThreadId := GetCurrentThreadId; @@ -1372,6 +1389,12 @@ function TGocciaAtomics.AtomicsXor(const AArgs: TGocciaArgumentsCollection; initialization InitCriticalSection(GAtomicsLock); + // Worker threads release their own Atomics waiters via the registry drain in + // ShutdownThreadRuntime. ShutdownAtomicsWaitersForCurrentThread removes only + // the calling thread's entries from the shared GAtomicsWaiters list; the + // main thread's all-threads teardown stays in this unit's finalization below + // (the per-thread/all-threads distinction is deliberate). + RegisterThreadvarCleanup(@ShutdownAtomicsWaitersForCurrentThread); finalization ShutdownAtomicsWaiters; diff --git a/source/units/Goccia.Builtins.DisposableStack.pas b/source/units/Goccia.Builtins.DisposableStack.pas index 39c7aab1c..8e86b0654 100644 --- a/source/units/Goccia.Builtins.DisposableStack.pas +++ b/source/units/Goccia.Builtins.DisposableStack.pas @@ -69,6 +69,7 @@ implementation Goccia.Error.Messages, Goccia.Error.Suggestions, Goccia.Scope.BindingMap, + Goccia.ThreadCleanupRegistry, Goccia.Values.Error, Goccia.Values.ErrorHelper, Goccia.Values.FunctionBase, @@ -565,8 +566,9 @@ procedure ClearDisposableStackSlotMap; end; initialization - -finalization - ClearDisposableStackSlotMap; + // FPC does not auto-finalize managed threadvars at thread exit. The registry + // drain releases this thread's slot map on worker exit (ShutdownThreadRuntime) + // and on the main thread (Goccia.ThreadCleanupRegistry's finalization). + RegisterThreadvarCleanup(@ClearDisposableStackSlotMap); end. diff --git a/source/units/Goccia.Builtins.Semver.pas b/source/units/Goccia.Builtins.Semver.pas index ca83e7927..ffd211381 100644 --- a/source/units/Goccia.Builtins.Semver.pas +++ b/source/units/Goccia.Builtins.Semver.pas @@ -26,6 +26,7 @@ implementation Goccia.Error.Suggestions, Goccia.ObjectModel, Goccia.Semver, + Goccia.ThreadCleanupRegistry, Goccia.Values.ArrayValue, Goccia.Values.ErrorHelper, Goccia.Values.NativeFunction, @@ -1489,8 +1490,9 @@ procedure ClearSemverHosts; initialization GSemverHosts := TSemverHostList.Create(True); - -finalization - ClearSemverHosts; + // FPC does not auto-finalize managed threadvars at thread exit. The registry + // drain frees this thread's host list on worker exit (ShutdownThreadRuntime) + // and on the main thread (Goccia.ThreadCleanupRegistry's finalization). + RegisterThreadvarCleanup(@ClearSemverHosts); end. diff --git a/source/units/Goccia.ImportMeta.pas b/source/units/Goccia.ImportMeta.pas index 255fa19a0..158e19e2f 100644 --- a/source/units/Goccia.ImportMeta.pas +++ b/source/units/Goccia.ImportMeta.pas @@ -34,6 +34,7 @@ implementation Goccia.Constants.PropertyNames, Goccia.Error.Messages, Goccia.Error.Suggestions, + Goccia.ThreadCleanupRegistry, Goccia.URI, Goccia.Values.ErrorHelper, Goccia.Values.NativeFunction; @@ -163,10 +164,12 @@ procedure ClearImportMetaCache; end; end; -finalization - // FPC does not auto-finalize managed threadvars at thread exit. Worker - // threads release this cache through ShutdownThreadRuntime; clear the main - // thread's copy on process shutdown too. - ClearImportMetaCache; +initialization + // FPC does not auto-finalize managed threadvars at thread exit. Register the + // cache clear so the registry drain releases this thread's copy on whichever + // thread tears down: a worker via ShutdownThreadRuntime, the main thread via + // Goccia.ThreadCleanupRegistry's finalization. (ClearImportMetaCache is also + // called directly from the engine's own teardown in Goccia.Engine.) + RegisterThreadvarCleanup(@ClearImportMetaCache); end. diff --git a/source/units/Goccia.RegExp.VM.pas b/source/units/Goccia.RegExp.VM.pas index 3d0bfc9b3..2b68e297c 100644 --- a/source/units/Goccia.RegExp.VM.pas +++ b/source/units/Goccia.RegExp.VM.pas @@ -22,9 +22,11 @@ function ExecuteRegExpVM(const AProgram: TRegExpProgram; const AInput: string; const AStartIndex: Integer; const ARequireStart: Boolean; out AResult: TRegExpVMResult): Boolean; -{ Release the per-thread input-decode memo. Called from this unit's - finalization (main thread) and from ShutdownThreadRuntime (worker threads), - because FPC does not auto-finalize managed threadvars at thread exit. } +{ Release the per-thread input-decode memo. Registered with + Goccia.ThreadCleanupRegistry from this unit's initialization, so the drain + releases it on worker exit (ShutdownThreadRuntime) and on the main thread (the + registry's finalization), because FPC does not auto-finalize managed + threadvars at thread exit. } procedure ClearRegExpInputMemo; implementation @@ -33,6 +35,7 @@ implementation TextSemantics, Goccia.RegExp.UnicodeData, + Goccia.ThreadCleanupRegistry, Goccia.Timeout; const @@ -1149,8 +1152,10 @@ function ExecuteRegExpVM(const AProgram: TRegExpProgram; end; end; -// FPC does not auto-finalize managed threadvars at thread exit; release the -// main-thread memo on shutdown so its retained subject/units are not leaked. +// FPC does not auto-finalize managed threadvars at thread exit; registered in +// this unit's initialization so the registry drain releases this thread's memo +// on worker exit and at main-thread shutdown, keeping its retained subject/units +// from leaking. procedure ClearRegExpInputMemo; begin GRegExpInputMemoStr := ''; @@ -1159,7 +1164,16 @@ procedure ClearRegExpInputMemo; GRegExpInputMemoValid := False; end; -finalization - ClearRegExpInputMemo; +initialization + // FPC does not auto-finalize managed threadvars at thread exit. Register this + // unit's regex-input memo, and also the is-ASCII memo owned by the shared + // TextSemantics unit: TextSemantics is generic infrastructure that stays free + // of engine dependencies, so its per-thread memo is registered here instead + // (every engine binary links the regex VM). See ClearAsciiMemo in + // source/shared/TextSemantics.pas. The registry drain releases both memos on + // worker exit (ShutdownThreadRuntime) and on the main thread + // (Goccia.ThreadCleanupRegistry's finalization). + RegisterThreadvarCleanup(@ClearRegExpInputMemo); + RegisterThreadvarCleanup(@ClearAsciiMemo); end. diff --git a/source/units/Goccia.Temporal.TimeZone.pas b/source/units/Goccia.Temporal.TimeZone.pas index 3f0755751..5b1b05a21 100644 --- a/source/units/Goccia.Temporal.TimeZone.pas +++ b/source/units/Goccia.Temporal.TimeZone.pas @@ -36,6 +36,7 @@ implementation SysUtils, Goccia.Temporal.Utils, Goccia.Temporal.TimeZoneData, + Goccia.ThreadCleanupRegistry, Goccia.Values.ErrorHelper, TimeZoneInformationFile, {$IFDEF UNIX} @@ -1980,15 +1981,18 @@ initialization CachedTimeZonePathCount := 0; CachedTimeZoneCaseCount := 0; CachedAvailablePrimaryTimeZoneIdentifiersLoaded := False; + // FPC does not auto-finalize managed threadvars at thread exit. The registry + // drain releases this thread's timezone cache on worker exit + // (ShutdownThreadRuntime) and on the main thread + // (Goccia.ThreadCleanupRegistry's finalization). + RegisterThreadvarCleanup(@ClearTimeZoneCache); {$IFDEF MSWINDOWS} InitCriticalSection(WindowsICUInitLock); {$ENDIF} finalization - // FPC does not auto-finalize managed threadvars at thread exit. Worker - // threads release the cache through ShutdownThreadRuntime; clear the main - // thread's copy on process shutdown too. - ClearTimeZoneCache; + // The timezone cache threadvars are released via Goccia.ThreadCleanupRegistry + // (registered above); only the Windows ICU init lock is torn down here. {$IFDEF MSWINDOWS} DoneCriticalSection(WindowsICUInitLock); {$ENDIF} diff --git a/source/units/Goccia.ThreadCleanupLeak.Test.pas b/source/units/Goccia.ThreadCleanupLeak.Test.pas index ceaf1bec6..9efbb4774 100644 --- a/source/units/Goccia.ThreadCleanupLeak.Test.pas +++ b/source/units/Goccia.ThreadCleanupLeak.Test.pas @@ -25,7 +25,15 @@ {$IFDEF UNIX}cthreads,{$ENDIF} SysUtils, + TextSemantics, + + Goccia.Builtins.Atomics, + Goccia.Builtins.DisposableStack, + Goccia.Builtins.Semver, Goccia.GarbageCollector, + Goccia.ImportMeta, + Goccia.RegExp.VM, + Goccia.Temporal.TimeZone, Goccia.ThreadCleanupRegistry, Goccia.Threading.Init, Goccia.Values.Primitives, @@ -39,12 +47,15 @@ TLeakTests = class(TTestSuite) procedure SetupTests; override; procedure TestDrainReclaimsMemberDefinitionThreadvars; + procedure TestMigratedCacheCleanupsAreRegistered; end; procedure TLeakTests.SetupTests; begin Test('draining the registry reclaims per-thread member-definition threadvars', TestDrainReclaimsMemberDefinitionThreadvars); + Test('each migrated per-thread cache/memo cleanup is registered with the registry', + TestMigratedCacheCleanupsAreRegistered); end; procedure TLeakTests.TestDrainReclaimsMemberDefinitionThreadvars; @@ -74,6 +85,25 @@ procedure TLeakTests.TestDrainReclaimsMemberDefinitionThreadvars; Expect((Populated - Drained) >= MIN_RECLAIMED_BYTES).ToBe(True); end; +procedure TLeakTests.TestMigratedCacheCleanupsAreRegistered; +begin + // Issue #893: each per-thread cache/memo that ShutdownThreadRuntime used to + // clear by an explicit call must instead register its clear with + // Goccia.ThreadCleanupRegistry from its owning unit's initialization (those + // units are pulled in via this program's uses clause). If a unit drops its + // RegisterThreadvarCleanup call, that threadvar silently leaks again on every + // worker-thread exit; pinning each registration here makes that regress + // loudly. (TestDrainReclaimsMemberDefinitionThreadvars proves the drain then + // runs the registered cleanups and reclaims their heap.) + Expect(IsThreadvarCleanupRegistered(@ClearImportMetaCache)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ShutdownAtomicsWaitersForCurrentThread)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ClearDisposableStackSlotMap)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ClearSemverHosts)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ClearTimeZoneCache)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ClearRegExpInputMemo)).ToBe(True); + Expect(IsThreadvarCleanupRegistered(@ClearAsciiMemo)).ToBe(True); +end; + begin // EnsureSharedPrototypesInitialized builds singletons whose getters assert they // were created on the main thread; pre-build them here first. diff --git a/source/units/Goccia.ThreadCleanupRegistry.pas b/source/units/Goccia.ThreadCleanupRegistry.pas index 39aa2cd00..099f573e0 100644 --- a/source/units/Goccia.ThreadCleanupRegistry.pas +++ b/source/units/Goccia.ThreadCleanupRegistry.pas @@ -23,8 +23,13 @@ interface type { Parameterless cleanup callback. Must release only the calling thread's own - managed threadvars (e.g. SetLength(FMembers, 0)); it runs on whichever - thread drains the registry, so it must not touch another thread's state. } + managed state — usually a managed threadvar (e.g. SetLength(FMembers, 0)), + but also a thread's own entries in a shared lock-guarded structure (e.g. the + Atomics waiter list, where each waiter is keyed by its owner thread id). It + runs on whichever thread drains the registry, so it must not touch another + thread's state, and — because the drain runs on both worker exit and + main-thread finalization — it must stay safe to call after that state has + already been torn down by a unit's own finalization. } TGocciaThreadvarCleanupProc = procedure; { Register a threadvar-cleanup callback. Call once per unit, from the unit's @@ -37,6 +42,11 @@ procedure RegisterThreadvarCleanup(const AProc: TGocciaThreadvarCleanupProc); thread). Safe to call multiple times and on any thread. } procedure RunThreadvarCleanups; +{ Returns True if AProc is currently registered. Intended for regression tests + that pin a specific unit's cleanup registration (so a dropped + RegisterThreadvarCleanup call fails loudly); not part of the teardown path. } +function IsThreadvarCleanupRegistered(const AProc: TGocciaThreadvarCleanupProc): Boolean; + implementation const @@ -72,6 +82,21 @@ procedure RunThreadvarCleanups; GCleanups[I](); end; +function IsThreadvarCleanupRegistered(const AProc: TGocciaThreadvarCleanupProc): Boolean; +var + I: Integer; +begin + // Compare the stored code pointers byte-for-byte. A direct `=` (or a Pointer + // cast) on a parameterless procedural variable makes FPC *call* it instead of + // reading its address; passing the procvars to CompareByte's untyped const + // params takes their addresses, so this reads the code pointers without + // invoking them. + for I := 0 to GCleanupCount - 1 do + if CompareByte(GCleanups[I], AProc, SizeOf(TGocciaThreadvarCleanupProc)) = 0 then + Exit(True); + Result := False; +end; + finalization RunThreadvarCleanups; diff --git a/source/units/Goccia.Threading.pas b/source/units/Goccia.Threading.pas index fe3fcb799..e69ce0fe1 100644 --- a/source/units/Goccia.Threading.pas +++ b/source/units/Goccia.Threading.pas @@ -187,19 +187,12 @@ implementation Math, SysUtils, - TextSemantics, TimingUtils, - Goccia.Builtins.Atomics, - Goccia.Builtins.DisposableStack, - Goccia.Builtins.Semver, Goccia.CallStack, Goccia.Coverage, Goccia.GarbageCollector, - Goccia.ImportMeta, Goccia.MicrotaskQueue, - Goccia.RegExp.VM, - Goccia.Temporal.TimeZone, Goccia.ThreadCleanupRegistry, Goccia.Values.Primitives; @@ -232,19 +225,17 @@ procedure ShutdownThreadRuntime; begin // Coverage tracker is NOT shut down here — the main thread reads it // after workers complete, then merges into the main tracker. - ClearImportMetaCache; - ShutdownAtomicsWaitersForCurrentThread; - ClearDisposableStackSlotMap; - ClearSemverHosts; - ClearTimeZoneCache; - // The #805/#806 memos already finalize on the main thread but, like the - // caches above, FPC does not release their managed threadvars on a worker - // thread exit — clear them here too. - ClearRegExpInputMemo; - ClearAsciiMemo; - // Drain the engine-wide threadvar-cleanup registry: every builtin's and - // value type's cached member-definition array registers its release proc in - // Goccia.ThreadCleanupRegistry. This releases this worker thread's copies. + // + // Drain the engine-wide threadvar-cleanup registry. Every per-thread managed + // cache and memo registers its release proc in Goccia.ThreadCleanupRegistry + // from its own unit initialization — the builtins' and value types' cached + // member-definition arrays, plus the import.meta, Atomics-waiter, + // disposable-stack, semver-host, timezone, regex-input and is-ASCII caches — + // so this single drain releases all of this worker thread's copies (the + // registry's own finalization does the same on the main thread). Run it + // before the ordered object-lifecycle shutdowns below so caches that touch + // the collector (e.g. import.meta unpinning) are released while the GC is + // still alive. RunThreadvarCleanups; TGocciaMicrotaskQueue.Shutdown; TGocciaCallStack.Shutdown; From f02fbe4681c6a52a32c2263cf946995b0f54bc79 Mon Sep 17 00:00:00 2001 From: Johannes Stein Date: Sun, 28 Jun 2026 22:01:44 +0100 Subject: [PATCH 2/2] docs(engine): correct stale regex-input memo threadvar comment (#893) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memo declaration comment still described the pre-registry lifecycle: it claimed the unit finalization clears the main-thread memo and that a worker thread's last-held pair is a bounded residual. After the #893 migration the memo is registered with Goccia.ThreadCleanupRegistry, so the drain clears it on worker exit and at main-thread shutdown — no residual. Comment-only. Co-Authored-By: Claude Opus 4.8 --- source/units/Goccia.RegExp.VM.pas | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source/units/Goccia.RegExp.VM.pas b/source/units/Goccia.RegExp.VM.pas index 2b68e297c..d1d488ad5 100644 --- a/source/units/Goccia.RegExp.VM.pas +++ b/source/units/Goccia.RegExp.VM.pas @@ -1067,10 +1067,11 @@ function RunVM(const AProgram: TRegExpProgram; const AInput: TRegExpInput; // so the hit check is O(1). Pure optimization — clearing it is always safe. // Single-entry: a different subject replaces the retained pair via managed // assignment (the prior string/array is released, so the cache never grows). -// FPC does not auto-finalize managed threadvars at thread exit, so the unit -// finalization below clears the main-thread memo on shutdown; a worker thread's -// last-held pair is a bounded residual, the same as the engine's other managed -// threadvars (e.g. each builtin's FStaticMembers). +// FPC does not auto-finalize managed threadvars at thread exit. ClearRegExpInputMemo +// is registered with Goccia.ThreadCleanupRegistry from this unit's initialization, +// so the registry drain releases each thread's pair on worker exit +// (ShutdownThreadRuntime) and the main thread's on process shutdown (the +// registry's finalization) — no thread retains a residual. threadvar GRegExpInputMemoStr: string; GRegExpInputMemoUnits: array of Cardinal;