merge: Campaign LA LA5 - plugin hosting review-closed

src/AcDream.Runtime/Entities/RuntimeEntityDirectory.cs

@@ -16,6 +16,7 @@ public sealed class RuntimeEntityDirectory
     public const uint LastLocalEntityId = 0x3FFF_FFFFu;
 
     private readonly InboundPhysicsStateController _inbound = new();
+    private readonly object _activeGate = new();
     private readonly Dictionary<uint, RuntimeEntityRecord> _activeByGuid = new();
     private readonly Dictionary<(uint Guid, ushort Incarnation), RuntimeEntityRecord>
         _teardownByIncarnation = new();
@@ -35,10 +36,27 @@ public sealed class RuntimeEntityDirectory
         _nextLocalEntityId = firstLocalEntityId;
     }
 
-    public int Count => _activeByGuid.Count;
+    public int Count
+    {
+        get
+        {
+            lock (_activeGate)
+                return _activeByGuid.Count;
+        }
+    }
     public int PendingTeardownCount => _teardownByIncarnation.Count;
-    public int ClaimedLocalIdCount => _byLocalId.Count;
+    public int ClaimedLocalIdCount
+    {
+        get
+        {
+            lock (_activeGate)
+                return _byLocalId.Count;
+        }
+    }
     public ulong SessionLifetimeVersion { get; private set; }
+    /// <summary>Update-thread-only borrowed collection. Cross-thread hosts use
+    /// <see cref="AcquireActiveRead"/> through <c>IRuntimeEntityView.Visit</c>
+    /// so membership cannot change during enumeration.</summary>
     public IReadOnlyCollection<RuntimeEntityRecord> ActiveRecords => _activeByGuid.Values;
     public IReadOnlyCollection<RuntimeEntityRecord> TeardownRecords =>
         _teardownByIncarnation.Values;
@@ -87,34 +105,48 @@ public sealed class RuntimeEntityDirectory
 
     public RuntimeEntityRecord AddActive(WorldSession.EntitySpawn snapshot)
     {
-        if (_activeByGuid.ContainsKey(snapshot.Guid))
+        lock (_activeGate)
         {
-            throw new InvalidOperationException(
-                $"Live entity 0x{snapshot.Guid:X8} already has an active incarnation.");
-        }
+            if (_activeByGuid.ContainsKey(snapshot.Guid))
+            {
+                throw new InvalidOperationException(
+                    $"Live entity 0x{snapshot.Guid:X8} already has an active incarnation.");
+            }
 
-        var record = new RuntimeEntityRecord(snapshot);
-        _activeByGuid.Add(snapshot.Guid, record);
-        try
-        {
-            ClaimLocalId(record);
-            return record;
-        }
-        catch
-        {
-            _activeByGuid.Remove(snapshot.Guid);
-            throw;
+            var record = new RuntimeEntityRecord(snapshot);
+            _activeByGuid.Add(snapshot.Guid, record);
+            try
+            {
+                ClaimLocalId(record);
+                return record;
+            }
+            catch
+            {
+                _activeByGuid.Remove(snapshot.Guid);
+                throw;
+            }
         }
     }
 
-    public bool RemoveActive(uint guid, out RuntimeEntityRecord? record) =>
-        _activeByGuid.Remove(guid, out record);
+    public bool RemoveActive(uint guid, out RuntimeEntityRecord? record)
+    {
+        lock (_activeGate)
+            return _activeByGuid.Remove(guid, out record);
+    }
 
     public bool RemoveActive(RuntimeEntityRecord expected)
     {
-        if (!IsCurrent(expected))
-            return false;
-        return _activeByGuid.Remove(expected.ServerGuid);
+        lock (_activeGate)
+        {
+            if (!_activeByGuid.TryGetValue(
+                    expected.ServerGuid,
+                    out RuntimeEntityRecord? current)
+                || !ReferenceEquals(current, expected))
+            {
+                return false;
+            }
+            return _activeByGuid.Remove(expected.ServerGuid);
+        }
     }
 
     public void RetainTeardown(RuntimeEntityRecord record)
@@ -157,46 +189,52 @@ public sealed class RuntimeEntityDirectory
 
     public uint ClaimLocalId(RuntimeEntityRecord record)
     {
-        if (!IsKnown(record))
+        lock (_activeGate)
         {
-            throw new InvalidOperationException(
-                "A local id can only be claimed for an active or retained incarnation.");
+            if (!IsKnown(record))
+            {
+                throw new InvalidOperationException(
+                    "A local id can only be claimed for an active or retained incarnation.");
+            }
+
+            if (record.LocalEntityId is { } existing)
+                return existing;
+
+            uint start = _nextLocalEntityId;
+            do
+            {
+                uint candidate = _nextLocalEntityId;
+                _nextLocalEntityId = candidate == LastLocalEntityId
+                    ? FirstLocalEntityId
+                    : candidate + 1u;
+                if (_byLocalId.ContainsKey(candidate))
+                    continue;
+
+                _byLocalId.Add(candidate, record);
+                record.LocalEntityId = candidate;
+                return candidate;
+            }
+            while (_nextLocalEntityId != start);
+
+            throw new InvalidOperationException("The live entity id namespace is exhausted.");
         }
-
-        if (record.LocalEntityId is { } existing)
-            return existing;
-
-        uint start = _nextLocalEntityId;
-        do
-        {
-            uint candidate = _nextLocalEntityId;
-            _nextLocalEntityId = candidate == LastLocalEntityId
-                ? FirstLocalEntityId
-                : candidate + 1u;
-            if (_byLocalId.ContainsKey(candidate))
-                continue;
-
-            _byLocalId.Add(candidate, record);
-            record.LocalEntityId = candidate;
-            return candidate;
-        }
-        while (_nextLocalEntityId != start);
-
-        throw new InvalidOperationException("The live entity id namespace is exhausted.");
     }
 
     public bool ReleaseLocalId(RuntimeEntityRecord record)
     {
-        if (record.LocalEntityId is not { } localId)
-            return false;
-        if (_byLocalId.TryGetValue(localId, out RuntimeEntityRecord? retained)
-            && ReferenceEquals(retained, record))
+        lock (_activeGate)
         {
-            _byLocalId.Remove(localId);
-        }
+            if (record.LocalEntityId is not { } localId)
+                return false;
+            if (_byLocalId.TryGetValue(localId, out RuntimeEntityRecord? retained)
+                && ReferenceEquals(retained, record))
+            {
+                _byLocalId.Remove(localId);
+            }
 
-        record.LocalEntityId = null;
-        return true;
+            record.LocalEntityId = null;
+            return true;
+        }
     }
 
     public ulong AdvanceLifetimeMutation(uint serverGuid)
@@ -219,15 +257,39 @@ public sealed class RuntimeEntityDirectory
 
     public bool CompleteSessionClearIfConverged()
     {
-        if (_activeByGuid.Count != 0 || _teardownByIncarnation.Count != 0)
-            return false;
+        lock (_activeGate)
+        {
+            if (_activeByGuid.Count != 0 || _teardownByIncarnation.Count != 0)
+                return false;
 
-        _byLocalId.Clear();
+            _byLocalId.Clear();
+        }
         ParentAttachments.Clear();
         _inbound.Clear();
         return true;
     }
 
+    /// <summary>
+    /// Enters the exact active-membership read boundary. Add/remove and local-id
+    /// membership commits serialize behind this allocation-free lease; record
+    /// ownership remains canonical here and no copied gameplay collection is
+    /// introduced.
+    /// </summary>
+    internal ActiveReadLease AcquireActiveRead() => new(_activeGate);
+
+    internal readonly struct ActiveReadLease : IDisposable
+    {
+        private readonly object _gate;
+
+        internal ActiveReadLease(object gate)
+        {
+            _gate = gate;
+            Monitor.Enter(gate);
+        }
+
+        public void Dispose() => Monitor.Exit(_gate);
+    }
+
     public void RefreshSnapshot(
         RuntimeEntityRecord record,
         WorldSession.EntitySpawn accepted,

src/AcDream.Runtime/Entities/RuntimeEntityObjectViews.cs

@@ -91,6 +91,8 @@ internal sealed class RuntimeEntityObjectViews
             uint serverGuid,
             out RuntimeEntitySnapshot entity)
         {
+            using RuntimeEntityDirectory.ActiveReadLease lease =
+                owner.AcquireActiveRead();
             if (owner.TryGetActive(
                     serverGuid,
                     out RuntimeEntityRecord record))
@@ -106,6 +108,8 @@ internal sealed class RuntimeEntityObjectViews
         public void Visit(IRuntimeEntityVisitor visitor)
         {
             ArgumentNullException.ThrowIfNull(visitor);
+            using RuntimeEntityDirectory.ActiveReadLease lease =
+                owner.AcquireActiveRead();
             foreach (RuntimeEntityRecord record in owner.ActiveRecords)
             {
                 RuntimeEntitySnapshot entity = Snapshot(record);

src/AcDream.Runtime/Session/SessionStatusWriter.cs

@@ -13,9 +13,9 @@ namespace AcDream.Runtime.Session;
 /// is a single shared-stdout JSONL diagnostics stream with no per-session
 /// file; this class writes one file per session, meant to be read by an
 /// external process (the launcher) rather than scraped from console output.
-/// Event shapes are versioned (<c>"v":1</c>) so a future event kind
-/// (<c>pluginLoaded</c>/<c>pluginFailed</c>, LA5) can be added without
-/// breaking an existing reader.
+/// Event shapes are versioned (<c>"v":1</c>); LA5's
+/// <c>pluginLoaded</c>/<c>pluginFailed</c> additions use that same envelope
+/// without breaking an existing reader.
 /// </para>
 ///
 /// <para>
@@ -29,9 +29,11 @@ namespace AcDream.Runtime.Session;
 /// </para>
 ///
 /// <para>
-/// <strong>Never write credential material into this stream.</strong> Every
-/// event method below takes only identifiers, names, and counts — there is no
-/// parameter shape that could carry a password, by construction.
+/// <strong>Never write credential material into this stream.</strong> LA5's
+/// <see cref="PluginFailed"/> diagnostic is caller-supplied text, so hosts may
+/// pass only the plugin lifecycle failure and must never append session
+/// credentials or other secrets. Neither plugin host exposes credentials
+/// through <c>IPluginHost</c>.
 /// </para>
 ///
 /// <para>
@@ -205,6 +207,27 @@ public sealed class SessionStatusWriter
             characterName,
         });
 
+    public void PluginLoaded(string sessionId, string plugin) =>
+        Write(new
+        {
+            v = VocabularyVersion,
+            e = "pluginLoaded",
+            t = Now(),
+            sessionId,
+            plugin,
+        });
+
+    public void PluginFailed(string sessionId, string plugin, string error) =>
+        Write(new
+        {
+            v = VocabularyVersion,
+            e = "pluginFailed",
+            t = Now(),
+            sessionId,
+            plugin,
+            error,
+        });
+
     public void Disconnected(string sessionId, string reason)
     {
         if (!IsEnabled)