From a9ccc5acf507a9329741d49c0801e8c2e0df48f8 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 21 May 2026 17:01:43 +0200 Subject: [PATCH] fix(O-T4): thread-safety lock in DatDatabaseWrapper + drop unused using MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review findings on T4: 1. Added lock(_lock) around _db.TryGet and TryGetFileBytes in DatDatabaseWrapper, matching WB's DefaultDatDatabase pattern. ObjectMeshManager.PrepareMeshDataAsync runs on the thread pool, so concurrent dat access through the adapter must be serialized — our underlying DatCollection is not documented as thread-safe. 2. Removed unused `using WorldBuilder.Shared.Models;` from WbMeshAdapter.cs (its only purpose was TerrainEntry, which moved to AcDream.Core in T2). Build green; tests green (1147 passing, 8 pre-existing failures baseline). Spec: docs/superpowers/specs/2026-05-21-phase-o-dat-path-unification-design.md Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Rendering/Wb/DatCollectionAdapter.cs | 28 ++++++++++++++----- src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs | 1 - 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/AcDream.App/Rendering/Wb/DatCollectionAdapter.cs b/src/AcDream.App/Rendering/Wb/DatCollectionAdapter.cs index 874fd33..3b661e2 100644 --- a/src/AcDream.App/Rendering/Wb/DatCollectionAdapter.cs +++ b/src/AcDream.App/Rendering/Wb/DatCollectionAdapter.cs @@ -118,6 +118,7 @@ internal sealed class DatDatabaseWrapper : IDatDatabase { private readonly DatDatabase _db; private readonly ConcurrentDictionary<(Type, uint), IDBObj> _cache = new(); + private readonly object _lock = new(); public DatDatabaseWrapper(DatDatabase db) { @@ -142,20 +143,33 @@ internal sealed class DatDatabaseWrapper : IDatDatabase return true; } - if (_db.TryGet(fileId, out value)) + lock (_lock) { - _cache.TryAdd((typeof(T), fileId), value); - return true; + if (_db.TryGet(fileId, out value)) + { + _cache.TryAdd((typeof(T), fileId), value); + return true; + } } return false; } - public bool TryGetFileBytes(uint fileId, [MaybeNullWhen(false)] out byte[] value) => - _db.TryGetFileBytes(fileId, out value); + public bool TryGetFileBytes(uint fileId, [MaybeNullWhen(false)] out byte[] value) + { + lock (_lock) + { + return _db.TryGetFileBytes(fileId, out value); + } + } - public bool TryGetFileBytes(uint fileId, ref byte[] bytes, out int bytesRead) => - _db.TryGetFileBytes(fileId, ref bytes, out bytesRead); + public bool TryGetFileBytes(uint fileId, ref byte[] bytes, out int bytesRead) + { + lock (_lock) + { + return _db.TryGetFileBytes(fileId, ref bytes, out bytesRead); + } + } public bool TrySave(T obj, int iteration = 0) where T : IDBObj => throw new NotSupportedException("DatDatabaseWrapper is read-only."); diff --git a/src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs b/src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs index b5c6a80..ed8b8ef 100644 --- a/src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs +++ b/src/AcDream.App/Rendering/Wb/WbMeshAdapter.cs @@ -10,7 +10,6 @@ using DatReaderWriter.DBObjs; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Silk.NET.OpenGL; -using WorldBuilder.Shared.Models; using WorldBuilder.Shared.Services; namespace AcDream.App.Rendering.Wb;