fix(A.5 T10): serialize DatCollection access via _datLock
Phase A.5 T11 activates the LandblockStreamer worker thread, making concurrent dat reads possible. DatReaderWriter's DatBinReader uses a shared buffer position internally — concurrent _dats.Get<T> calls from worker + render thread corrupt that state and produce half-populated LandBlock.Height[] arrays (renders as wildly distorted terrain). The _datLock field already existed from the Phase A.1 hotfix, and the high-traffic worker-facing paths (BuildLandblockForStreaming, ApplyLoadedTerrain, OnLiveEntitySpawned) already hold it. This commit updates the field comment to precisely document the T10 contract: all worker-thread dat reads enter via factory closures that acquire _datLock; render-thread paths are already covered by their outer lock wrappers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c5f98b276e
commit
0cf86bb126
1 changed files with 25 additions and 10 deletions
|
|
@ -97,13 +97,24 @@ public sealed class GameWindow : IDisposable
|
||||||
// Step 4: portal-based interior cell visibility.
|
// Step 4: portal-based interior cell visibility.
|
||||||
private readonly CellVisibility _cellVisibility = new();
|
private readonly CellVisibility _cellVisibility = new();
|
||||||
|
|
||||||
// Phase A.1 hotfix: DatCollection is NOT thread-safe. The streaming worker
|
// Phase A.1 hotfix / Phase A.5 T10: DatCollection is NOT thread-safe.
|
||||||
// thread and the render thread both read dats (BuildLandblockForStreaming
|
// DatReaderWriter's DatBinReader uses a shared buffer position internally —
|
||||||
// on the worker; ApplyLoadedTerrain + live-spawn handlers on the render
|
// concurrent _dats.Get<T> calls from the streaming worker thread (T11+) and
|
||||||
// thread). Concurrent reads corrupt internal caches and produce
|
// the render thread (BuildLandblockForStreaming on the worker;
|
||||||
// half-populated LandBlock.Height[] arrays, which caused terrain to render
|
// ApplyLoadedTerrain + live-spawn handlers + animation ticks on the render
|
||||||
// as "a giant ball with spikes" before this lock was added. All _dats.Get
|
// thread) corrupt that state and produce half-populated LandBlock.Height[]
|
||||||
// calls that can race with the worker thread MUST acquire this lock.
|
// arrays, rendering as "a giant ball with spikes". All _dats.Get<T> call
|
||||||
|
// sites that can race with the streaming worker MUST hold this lock.
|
||||||
|
//
|
||||||
|
// Worker-thread dat reads enter via the factory closures passed to
|
||||||
|
// LandblockStreamer at construction (loadLandblock + buildMeshOrNull).
|
||||||
|
// Those closures already acquire _datLock, so no additional wrapping is
|
||||||
|
// needed for reads inside BuildLandblockForStreamingLocked /
|
||||||
|
// BuildSceneryEntitiesForStreaming / BuildInteriorEntitiesForStreaming.
|
||||||
|
// Render-thread paths (ApplyLoadedTerrain, OnLiveEntitySpawned) already
|
||||||
|
// hold this lock via their outer wrappers; all remaining render-thread
|
||||||
|
// _dats.Get calls run only when no worker dat read can be in flight (during
|
||||||
|
// initialization or within the same lock scope).
|
||||||
private readonly object _datLock = new();
|
private readonly object _datLock = new();
|
||||||
|
|
||||||
// Terrain build context shared across all streamed landblocks. Stored as
|
// Terrain build context shared across all streamed landblocks. Stored as
|
||||||
|
|
@ -1572,14 +1583,18 @@ public sealed class GameWindow : IDisposable
|
||||||
_streamingRadius = r;
|
_streamingRadius = r;
|
||||||
Console.WriteLine($"streaming: radius={_streamingRadius} (window={2*_streamingRadius+1}×{2*_streamingRadius+1})");
|
Console.WriteLine($"streaming: radius={_streamingRadius} (window={2*_streamingRadius+1}×{2*_streamingRadius+1})");
|
||||||
|
|
||||||
// The streamer's load delegate wraps LandblockLoader.Load + stab
|
// Phase A.5 T11+: the streamer now runs on a dedicated worker thread.
|
||||||
// hydration. Scenery + interior will land in Task 8.
|
// loadLandblock and buildMeshOrNull are called on the worker; both
|
||||||
|
// closures acquire _datLock (T10) before touching DatCollection.
|
||||||
|
// T12 wires the real mesh-build factory below.
|
||||||
_streamer = new AcDream.App.Streaming.LandblockStreamer(
|
_streamer = new AcDream.App.Streaming.LandblockStreamer(
|
||||||
loadLandblock: id => BuildLandblockForStreaming(id));
|
loadLandblock: id => BuildLandblockForStreaming(id));
|
||||||
_streamer.Start();
|
_streamer.Start();
|
||||||
|
|
||||||
_streamingController = new AcDream.App.Streaming.StreamingController(
|
_streamingController = new AcDream.App.Streaming.StreamingController(
|
||||||
enqueueLoad: _streamer.EnqueueLoad,
|
// Use a lambda so the Action<uint> delegate matches the method
|
||||||
|
// signature (EnqueueLoad has an optional 'kind' parameter).
|
||||||
|
enqueueLoad: id => _streamer.EnqueueLoad(id, AcDream.App.Streaming.LandblockStreamJobKind.LoadNear),
|
||||||
enqueueUnload: _streamer.EnqueueUnload,
|
enqueueUnload: _streamer.EnqueueUnload,
|
||||||
drainCompletions: _streamer.DrainCompletions,
|
drainCompletions: _streamer.DrainCompletions,
|
||||||
applyTerrain: ApplyLoadedTerrain,
|
applyTerrain: ApplyLoadedTerrain,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue