fix(gc): make BytesAllocated reads tear-free on 32-bit targets

docs/garbage-collector.md

@@ -158,6 +158,8 @@ Two kinds of lock coordinate the collectors, with a strict order between them:
 - **The global collect lock** serializes `Collect`/`CollectYoung` across all collectors — the mark epoch (`GCCurrentMark`) is process-global and some intrinsic objects are shared, so two concurrent collections advancing the epoch mid-mark would unmark each other's live sets.
 - **A per-collector accounting lock** is a short-lived leaf lock for the byte counters a *foreign* thread can drive (`BytesAllocated`, the external-byte total, the forced-collect floor, the peak). The one cross-thread entry point into a collector is `ReleaseExternalBytes` through an error object's reserving-collector pointer (an error charged on collector A can be destroyed on worker thread B); everything else reaches a collector through the thread-local `Instance`, so the managed-object list and root sets are owner-thread-confined and unlocked. Keeping the counters on their own per-collector lock means one worker's full mark-and-sweep does not stall any other worker's per-allocation register/unregister — and workers never contend with each other on the allocation path at all, only with the rare cross-thread release aimed at their own collector.
 
+Readers of those counters do not take the accounting lock on 64-bit targets: every write goes through the lock, and an aligned `Int64` load cannot tear there, so `BytesAllocated` stays a bare field read on the per-allocation path. On 32-bit targets a cross-thread `ReleaseExternalBytes` splits its write into two stores, so the `BytesAllocated` accessor takes the lock for the load — no 64-bit interlocked read is available, since FPC 3.2.2 declares those only under `CPU64` and CI also builds i386-win32. The peak and lifetime totals are written only by the owning thread and need neither treatment.
+
 The order is collect lock → accounting lock, never the reverse (and never two accounting locks at once): a sweep settles its freed bytes — and a swept error destructor releases its cross-collector charge — under an accounting lock while the collect lock is held, and the reservation path drops the accounting lock before it collects. The accounting lock lives and dies with its collector; a cross-thread release entering it is safe on the same terms as the counter fields it guards, per the lifecycle invariant that a charged object is freed before its reserving collector shuts down.
 
 Key behavior on worker threads:

source/units/Goccia.GarbageCollector.pas

@@ -163,6 +163,15 @@   TGarbageCollector = class
     // shared cache line entirely: each worker contends only with the rare
     // cross-thread release actually aimed at its collector.
     //
+    // Readers: every write to these counters goes through this lock, so a
+    // reader only has to avoid observing a half-applied cross-thread write.
+    // On 64-bit an aligned Int64 load cannot tear, so readers take nothing and
+    // BytesAllocated stays a bare field load on the per-allocation path; on
+    // 32-bit ReleaseExternalBytes splits its write into two stores and readers
+    // go through GetBytesAllocated, which takes this lock. Code that already
+    // holds the lock reads the fields directly — GetBytesAllocated must never
+    // be called from inside a section.
+    //
     // Lock order: the global GCCollectLock may be held when taking an
     // accounting lock (a sweep's aggregate byte settle, a swept destructor's
     // cross-collector release). NEVER take GCCollectLock — or a second
@@ -210,8 +219,19 @@   TGarbageCollector = class
 
     function GetManagedObjectCount: Integer;
     function GetWatermark: Integer; {$IFDEF FPC}inline;{$ENDIF}
+    // Untorn read of the live-byte total for callers that hold no lock — see
+    // the reader rule at FAccountingLock. Never call it while holding the
+    // accounting lock; read FBytesAllocated directly there.
+    function GetBytesAllocated: Int64; {$IFDEF FPC}inline;{$ENDIF}
+    // Pressure predicate over an already-read live total, so the charge path
+    // can decide from the value it just committed instead of re-reading it
+    // through the locked accessor with the lock held.
+    function NeedsMemoryPressureCollection(
+      const ABytesAllocated: Int64): Boolean; overload;
     procedure ClearActiveRootEntries(const AObject: TGCManagedObject);
     procedure GrowActiveRootStack;
+    // Reads the live total and the forced-collect floor, so the caller must
+    // hold FAccountingLock across the decision it feeds.
     function ShouldForceLimitCollection(const ABytes: Int64): Boolean;
     // The one --max-memory fit predicate (overflow guard + ceiling test).
     // Caller must hold FAccountingLock ("Locked" suffix).
@@ -274,7 +294,7 @@   TGarbageCollector = class
     // active root stack so a stack-held object survives the collection.
     procedure CollectIfNeeded(const AProtect: TGCManagedObject); overload;
 
-    function NeedsMemoryPressureCollection: Boolean;
+    function NeedsMemoryPressureCollection: Boolean; overload;
 
     // Collects when pressure has been latched by an external reservation or
     // when the live set has crossed the reserve below the ceiling. AForce
@@ -336,7 +356,12 @@   TGarbageCollector = class
     // number of bytes currently tracked by the GC (InstanceSize per
     // registered object). Set MaxBytes to a positive value to impose
     // a ceiling; allocations that exceed it raise a RangeError.
-    property BytesAllocated: Int64 read FBytesAllocated;
+    //
+    // BytesAllocated reads through an accessor because it is the one counter a
+    // foreign thread writes (ReleaseExternalBytes). The peak and the lifetime
+    // total are written only by the owning thread, so no reader of theirs can
+    // catch a half-applied write and they stay direct field reads.
+    property BytesAllocated: Int64 read GetBytesAllocated;
     property PeakBytesAllocated: Int64 read FPeakBytesAllocated;
     property TotalBytesAllocated: Int64 read FTotalBytesAllocated;
     property MaxBytes: Int64 read FMaxBytes write FMaxBytes;
@@ -1142,7 +1167,35 @@ procedure TGarbageCollector.CollectIfNeeded(
   end;
 end;
 
+function TGarbageCollector.GetBytesAllocated: Int64;
+begin
+  {$IF SizeOf(Pointer) >= 8}
+  Result := FBytesAllocated;
+  {$ELSE}
+  // A 32-bit target splits the 64-bit write in a cross-thread
+  // ReleaseExternalBytes into two stores, and a load that lands between them
+  // yields a total no memory-limit decision may be made on. No 64-bit
+  // interlocked load is available: FPC 3.2.2 declares those only under CPU64,
+  // and CI builds i386-win32. Nothing between the two calls can raise, so the
+  // load needs no exception frame — which also keeps this accessor inlinable.
+  CriticalSectionEnter(FAccountingLock);
+  Result := FBytesAllocated;
+  CriticalSectionLeave(FAccountingLock);
+  {$ENDIF}
+end;
+
 function TGarbageCollector.NeedsMemoryPressureCollection: Boolean;
+begin
+  // Guards before the counter read: on 32-bit GetBytesAllocated takes the
+  // accounting lock, and the periodic VM/interpreter pressure polls must stay
+  // free when no limit is set or a collection is already underway.
+  if (FMaxBytes <= 0) or FCollecting or FMemoryLimitFiring then
+    Exit(False);
+  Result := NeedsMemoryPressureCollection(GetBytesAllocated);
+end;
+
+function TGarbageCollector.NeedsMemoryPressureCollection(
+  const ABytesAllocated: Int64): Boolean;
 var
   Reserve: Int64;
 begin
@@ -1160,7 +1213,7 @@ function TGarbageCollector.NeedsMemoryPressureCollection: Boolean;
   if Reserve >= FMaxBytes then
     Reserve := FMaxBytes div 2;
 
-  Result := FBytesAllocated >= (FMaxBytes - Reserve);
+  Result := ABytesAllocated >= (FMaxBytes - Reserve);
 end;
 
 procedure TGarbageCollector.CollectForMemoryPressure(
@@ -1403,7 +1456,7 @@ function TGarbageCollector.TryChargeExternalBytesLocked(
     FPeakBytesAllocated := FBytesAllocated;
   if (FExternalBytesAllocatedSinceGC >=
       EXTERNAL_MEMORY_PRESSURE_ALLOCATION_INTERVAL) or
-     NeedsMemoryPressureCollection then
+     NeedsMemoryPressureCollection(FBytesAllocated) then
   begin
     FExternalPressurePending := True;
     if Assigned(FMemoryPressureCountdown) then

source/units/Goccia.MemoryLimit.pas

@@ -130,6 +130,7 @@ constructor TGocciaMemoryLimitError.Create(const ARequestedBytes,
 function CanAllocateNativeBytes(const ABytes: Int64): Boolean;
 var
   GC: TGarbageCollector;
+  LiveBytes: Int64;
 begin
   if ABytes <= 0 then
     Exit(True);
@@ -138,11 +139,15 @@ function CanAllocateNativeBytes(const ABytes: Int64): Boolean;
     one are unbounded by construction, and that is their choice to make. }
   if not Assigned(GC) or (GC.MaxBytes <= 0) then
     Exit(True);
+  { One snapshot for both tests: a cross-thread release between two reads
+    would split the decision across totals, and on 32-bit each read is a
+    locked load. }
+  LiveBytes := GC.BytesAllocated;
   { Overflow guard first: a JS-controlled length can multiply into a value
     that wraps, and a wrapped total would compare as comfortably in budget. }
-  if GC.BytesAllocated > High(Int64) - ABytes then
+  if LiveBytes > High(Int64) - ABytes then
     Exit(False);
-  Result := GC.BytesAllocated + ABytes <= GC.MaxBytes;
+  Result := LiveBytes + ABytes <= GC.MaxBytes;
 end;
 
 procedure RequireNativeBytes(const ABytes: Int64);