Snapshot VoiceAttack payload data to prevent concurrency-related serialization errors

Author: Tkael

IPC_Service/Messaging/MessageSerializer.cs

@@ -1,3 +1,5 @@
+#nullable enable
+
 using EddiIPC_Service.Messages;
 using Newtonsoft.Json;
 using System;
@@ -42,7 +44,10 @@ public static string Serialize ( MessageEnvelope message )
 
         try
         {
-            var json = JsonConvert.SerializeObject( message, new JsonSerializerSettings
+            // Create a snapshot of the message to avoid collection modification during serialization
+            var messageCopy = CreateMessageSnapshot( message );
+
+            var json = JsonConvert.SerializeObject( messageCopy, new JsonSerializerSettings
             {
                 NullValueHandling = NullValueHandling.Ignore,
                 DateFormatString = "O",
@@ -57,6 +62,56 @@ public static string Serialize ( MessageEnvelope message )
             Logging.Error( $"Failed to serialize message of type {message.Type}: {ex.Message}" );
             throw new ArgumentException( $"Message serialization failed: {ex.Message}", ex );
         }
+        catch ( InvalidOperationException ex )
+        {
+            Logging.Error( $"Failed to create stable snapshot for message type {message.Type}: {ex.Message}" );
+            throw new ArgumentException( $"Message serialization failed due to concurrent collection modification: {ex.Message}", ex );
+        }
+    }
+
+    /// <summary>
+    /// Creates a snapshot of the message with complete deep copy of all data.
+    /// Uses reflection-based deep copy to ensure thread-safety and fidelity.
+    /// NOTE: This is a second line of defense. A shallow snapshot is created at the source
+    /// (in VoiceAttackVariables.DispatchRuntimeEventPayload). This deep snapshot here provides
+    /// defense-in-depth using the robust Copy() extension from ObjectExtensions.
+    /// </summary>
+    private static MessageEnvelope CreateMessageSnapshot( MessageEnvelope message )
+    {
+        if ( message.Data is EventData eventData )
+        {
+            Dictionary<string, object>? payloadSnapshot = null;
+            if ( eventData.EventPayload != null )
+            {
+                try
+                {
+                    // The Copy() extension handles all collection types, circular references,
+                    // and nested structures automatically via reflection-based deep cloning
+                    payloadSnapshot = eventData.EventPayload.Copy();
+                }
+                catch ( Exception ex )
+                {
+                    Logging.Error( $"Failed to deep copy event payload: {ex.Message}" );
+                    throw new ArgumentException( "Message serialization failed: Cannot create stable snapshot", ex );
+                }
+            }
+
+            return new MessageEnvelope
+            {
+                Type = message.Type,
+                Id = message.Id,
+                Timestamp = message.Timestamp,
+                Data = new EventData
+                {
+                    EventType = eventData.EventType,
+                    EventName = eventData.EventName,
+                    EventPayload = payloadSnapshot
+                }
+            };
+        }
+
+        // For other data types, return the message as-is
+        return message;
     }
 
     /// <summary>

Tests/EddiVoiceAttackService/EndToEndIntegrationTests.cs

@@ -525,22 +525,28 @@ public async Task E2E_Reconnection_AfterDisconnect()
         public async Task E2E_InvalidConfigFile_ProperError()
         {
             // Arrange
-            var invalidPath = Path.Combine(Path.GetTempPath(), "invalid_config.json");
+            var invalidPath = Path.Combine(Path.GetTempPath(), $"invalid_config_{Guid.NewGuid():N}.json");
+            File.WriteAllText(invalidPath, "invalid json content");
             var pluginClient = new VoiceAttackPluginClient(invalidPath);
 
             // Act & Assert
             try
             {
                 await pluginClient.InitializeAsync( TestContext.CancellationToken ).ConfigureAwait( false );
-                Assert.Fail("Should have thrown FileNotFoundException");
+                Assert.Fail("Should have thrown JsonException or ArgumentException");
             }
-            catch (FileNotFoundException)
+            catch (JsonException)
             {
-                // Expected
+                // Expected - invalid JSON
+            }
+            catch (ArgumentException)
+            {
+                // Also acceptable - missing required properties
             }
             finally
             {
                 pluginClient.Dispose();
+                try { File.Delete(invalidPath); } catch { /* Ignore */ }
             }
         }
 

Tests/EddiVoiceAttackService/MessageSerializerTests.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 
 namespace Tests.EddiVoiceAttackService
 {
@@ -16,8 +17,9 @@ namespace Tests.EddiVoiceAttackService
     public class MessageSerializerTests
     {
         private const string TestMessageId = "12345678-1234-1234-1234-123456789012";
+        public TestContext TestContext { get; set; }
 
-        [ TestMethod ]
+        [TestMethod ]
         public void Serialize_DisconnectMessage_ProducesLengthPrefixedJson ()
         {
             // Arrange
@@ -206,29 +208,58 @@ public void Serialize_EventMessage_IncludesEventData ()
         }
 
         [ TestMethod ]
-        public void Serialize_CommandMessage_IncludesCommandData ()
+        public void Serialize_EventMessage_WithModifyingCollection_DoesNotThrow ()
         {
-            // Arrange
+            // Arrange: Create a mutable dictionary that simulates concurrent modification
+            var payload = new Dictionary<string, object> { { "key1", "value1" } };
             var envelope = new MessageEnvelope
             {
-                Type = "Command",
+                Type = "Event",
                 Timestamp = "2025-01-20T15:30:45.123Z",
                 Id = TestMessageId,
-                Data = new CommandData
+                Data = new EventData
                 {
-                    Command = "enable_monitor",
-                    Target = "Journal Monitor",
-                    Parameters = new Dictionary<string, object> { { "debug", true } }
+                    EventType = "TestEvent",
+                    EventName = "Test",
+                    EventPayload = payload
                 }
             };
 
-            // Act
-            var serialized = MessageSerializer.Serialize( envelope );
-            var deserialized = MessageSerializer.Deserialize( serialized );
+            // Act: Modify the dictionary in a separate task while serializing
+            var serializationTask = Task.Run( () =>
+            {
+                // Serialize multiple times to increase chance of collision
+                for ( var i = 0; i < 10; i++ )
+                {
+                    var serialized = MessageSerializer.Serialize( envelope );
+                    Assert.IsNotNull( serialized );
+                }
+            }, TestContext.CancellationToken );
 
-            // Assert
-            Assert.AreEqual( "Command", deserialized.Type );
-            Assert.IsNotNull( deserialized.Data );
+            var modificationTask = Task.Run( () =>
+            {
+                // Modify the dictionary while serialization is happening
+                for ( var i = 0; i < 100; i++ )
+                {
+                    payload[ $"key{i}" ] = $"value{i}";
+                    if ( (i % 10) == 0 )
+                    {
+                        Task.Delay( 1, TestContext.CancellationToken ).Wait(TestContext.CancellationToken);  // Brief pause to increase race condition likelihood
+                    }
+                }
+            }, TestContext.CancellationToken );
+
+            // Assert: Both tasks should complete without exception
+            try
+            {
+                Task.WaitAll( [ serializationTask, modificationTask ], TimeSpan.FromSeconds( 5 ) );
+                Assert.IsTrue( serializationTask.IsCompletedSuccessfully );
+                Assert.IsTrue( modificationTask.IsCompletedSuccessfully );
+            }
+            catch ( AggregateException ex )
+            {
+                Assert.Fail( $"Serialization failed during concurrent modification: {ex.InnerException?.Message}" );
+            }
         }
 
         [ TestMethod ]

VoiceAttackResponder/VoiceAttackVariables.cs

@@ -855,11 +855,16 @@ private static void DispatchRuntimeEventPayload( Dictionary<string, object> payl
         {
             try
             {
+                // Create a snapshot of the payload to ensure it's not modified during serialization/transmission
+                // This prevents "Collection was modified" exceptions that can occur when background threads
+                // update the variable dictionary while we're serializing the message
+                var payloadSnapshot = new Dictionary<string, object>( payload );
+
                 var eventData = new EventData
                 {
                     EventType = RuntimeEventType,
                     EventName = CommandActionEventName,
-                    EventPayload = payload
+                    EventPayload = payloadSnapshot
                 };
 
                 RuntimeEventDispatcher.DispatchAsync( eventData )