fix(gfx): serialize GetOrCreate/Register/Release on _lock (compositor race)
Once sleep paces the VM thread, the main-thread compositor's SnapshotVisibleObjects truly overlaps VM-thread _objects/_registry writes. GetOrCreate/Register/Release were unlocked -> 'Destination array is not long enough' under concurrent enumeration. _lock is re-entrant so BindDraw/EraseRange (already locked) stay correct. 52 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
28
engine/Age.Engine.Tests/GfxStateConcurrencyTests.cs
Normal file
28
engine/Age.Engine.Tests/GfxStateConcurrencyTests.cs
Normal file
@@ -0,0 +1,28 @@
|
|||||||
|
using System.Threading.Tasks;
|
||||||
|
using Age.Engine.Model;
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
public class GfxStateConcurrencyTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Snapshot_DoesNotThrow_WhileObjectsMutate()
|
||||||
|
{
|
||||||
|
var gfx = new GfxState();
|
||||||
|
var stop = false;
|
||||||
|
var writer = Task.Run(() =>
|
||||||
|
{
|
||||||
|
long h = 0;
|
||||||
|
while (!stop)
|
||||||
|
{
|
||||||
|
h = (h + 1) % 64;
|
||||||
|
gfx.BindDraw(h, 0, 0, 0, 10, 10, 0, 0); // GetOrCreate + visible
|
||||||
|
gfx.GetOrCreate(h + 100); // bare create
|
||||||
|
if (h % 8 == 0) gfx.Release(h + 100); // remove
|
||||||
|
}
|
||||||
|
});
|
||||||
|
// Hammer the reader concurrently; a dictionary mutated during enumeration would throw here.
|
||||||
|
for (int i = 0; i < 20000; i++) { var _ = gfx.SnapshotVisibleObjects(); }
|
||||||
|
stop = true;
|
||||||
|
writer.Wait();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -77,16 +77,22 @@ public sealed class GfxState
|
|||||||
}
|
}
|
||||||
|
|
||||||
public GfxObject GetOrCreate(long handle)
|
public GfxObject GetOrCreate(long handle)
|
||||||
|
{
|
||||||
|
// Locked: called from the VM thread (directly by 0x217/0x219/0x1ff/0x212/0x213 and inside BindDraw/anim
|
||||||
|
// ops) while the main-thread compositor enumerates _objects in SnapshotVisibleObjects. _lock is re-entrant
|
||||||
|
// (Monitor) so the callers that already hold it are fine.
|
||||||
|
lock (_lock)
|
||||||
{
|
{
|
||||||
if (!_objects.TryGetValue(handle, out var o)) { o = new GfxObject(); _objects[handle] = o; }
|
if (!_objects.TryGetValue(handle, out var o)) { o = new GfxObject(); _objects[handle] = o; }
|
||||||
CurrentObject = handle;
|
CurrentObject = handle;
|
||||||
return o;
|
return o;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Op 0x1a2 (gfx-cmd-register, native FUN_0042d360 -> FUN_0042cf70 hash insert): add the handle to
|
/// <summary>Op 0x1a2 (gfx-cmd-register, native FUN_0042d360 -> FUN_0042cf70 hash insert): add the handle to
|
||||||
/// the op-0x215 query registry. Native inserts map[handle]=handle; QuerySlot returns that value (handle) or
|
/// the op-0x215 query registry. Native inserts map[handle]=handle; QuerySlot returns that value (handle) or
|
||||||
/// -1. Only this op populates the query registry — geometry/draw ops do not.</summary>
|
/// -1. Only this op populates the query registry — geometry/draw ops do not.</summary>
|
||||||
public void Register(long handle) => _registry.Add(handle);
|
public void Register(long handle) { lock (_lock) { _registry.Add(handle); } }
|
||||||
|
|
||||||
public GfxObject? TryGet(long handle) => _objects.TryGetValue(handle, out var o) ? o : null;
|
public GfxObject? TryGet(long handle) => _objects.TryGetValue(handle, out var o) ? o : null;
|
||||||
|
|
||||||
@@ -96,10 +102,13 @@ public sealed class GfxState
|
|||||||
public long QueryField(long idx) => _fieldTable.TryGetValue(idx, out var v) ? v : 0;
|
public long QueryField(long idx) => _fieldTable.TryGetValue(idx, out var v) ? v : 0;
|
||||||
|
|
||||||
public void Release(long handle)
|
public void Release(long handle)
|
||||||
|
{
|
||||||
|
lock (_lock) // re-entrant: EraseRange already holds _lock; op 0x1fa calls this directly
|
||||||
{
|
{
|
||||||
_objects.Remove(handle);
|
_objects.Remove(handle);
|
||||||
_registry.Remove(handle); // op 0x1fa/0x1f7 also tear down the query registration
|
_registry.Remove(handle); // op 0x1fa/0x1f7 also tear down the query registration
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Op 0x1f7 semantics (native gfx_registry_erase_range @0x47d8b0): erase handles in
|
/// <summary>Op 0x1f7 semantics (native gfx_registry_erase_range @0x47d8b0): erase handles in
|
||||||
/// [handle, handle+count) when count>1, else just <paramref name="handle"/>. It is a teardown/erase,
|
/// [handle, handle+count) when count>1, else just <paramref name="handle"/>. It is a teardown/erase,
|
||||||
|
|||||||
Reference in New Issue
Block a user