Add lock around in-memory hits collection

Closes #1053

Author: openbullet

OpenBullet2.Native/ViewModels/MultiRunJobViewerViewModel.cs

@@ -342,11 +342,12 @@ private void UpdateBots()
 
     private void UpdateHitsCollection()
     {
+        var hitsSnapshot = MultiRunJob.GetHitsSnapshot();
         var hits = HitsFilter switch
         {
-            HitsFilter.Hits => MultiRunJob.Hits.Where(h => h.Type == "SUCCESS"),
-            HitsFilter.ToCheck => MultiRunJob.Hits.Where(h => h.Type == "NONE"),
-            HitsFilter.Custom => MultiRunJob.Hits.Where(h => h.Type != "SUCCESS" && h.Type != "NONE"),
+            HitsFilter.Hits => hitsSnapshot.Where(h => h.Type == "SUCCESS"),
+            HitsFilter.ToCheck => hitsSnapshot.Where(h => h.Type == "NONE"),
+            HitsFilter.Custom => hitsSnapshot.Where(h => h.Type != "SUCCESS" && h.Type != "NONE"),
             _ => throw new NotImplementedException()
         };
 

OpenBullet2.Web/Controllers/JobController.cs

@@ -544,7 +544,7 @@ public ActionResult<MrjHitLogDto> GetHitLog(int jobId, string hitId)
                 $"The job with id {jobId} is not a multi run job");
         }
 
-        var hit = mrJob.Hits.Find(h => h.Id == hitId) ?? throw new EntryNotFoundException(ErrorCode.HitNotFound,
+        var hit = mrJob.FindHit(hitId) ?? throw new EntryNotFoundException(ErrorCode.HitNotFound,
                 hitId, nameof(MultiRunJob.Hits));
         return new MrjHitLogDto { Log = hit.BotLogger?.Entries.ToList() };
     }
@@ -968,6 +968,8 @@ private async Task<MultiRunJobDto> MapMultiRunJobDto(MultiRunJob job, Cancellati
             _ => throw new NotImplementedException()
         };
 
+        var hits = job.GetHitsSnapshot();
+
         return new MultiRunJobDto
         {
             Id = job.Id,
@@ -1018,7 +1020,7 @@ job.Config is not null
             Elapsed = job.Elapsed,
             Remaining = job.Remaining,
             Progress = job.Progress < 0 ? 0 : job.Progress,
-            Hits = job.Hits.Select(h => new MrjHitDto
+            Hits = hits.Select(h => new MrjHitDto
             {
                 Id = h.Id,
                 Date = h.Date,

RuriLib.Tests/Models/Jobs/MultiRunJobTests.cs

@@ -3,6 +3,7 @@
 using RuriLib.Models.Jobs;
 using RuriLib.Models.Jobs.StartConditions;
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Reflection;
 using System.Runtime.CompilerServices;
@@ -159,6 +160,54 @@ public async Task Start_AfterCompletedRun_RestartsFromBeginning()
         Assert.Equal(0, job.Skip);
     }
 
+    [Fact]
+    public async Task GetHitsSnapshot_DuringConcurrentWrites_DoesNotThrow()
+    {
+        var job = CreateJob();
+        var hits = job.Hits;
+        var hitsLock = typeof(MultiRunJob)
+            .GetField("hitsLock", BindingFlags.Instance | BindingFlags.NonPublic)!
+            .GetValue(job)!;
+        using var cts = new CancellationTokenSource();
+
+        var writer = Task.Run(async () =>
+        {
+            var index = 0;
+
+            while (!cts.Token.IsCancellationRequested)
+            {
+                lock (hitsLock)
+                {
+                    hits.Add(CreateHit(index++));
+                }
+
+                await Task.Yield();
+            }
+        }, TestCancellationToken);
+
+        for (var i = 0; i < 200; i++)
+        {
+            var exception = Record.Exception(() => job.GetHitsSnapshot());
+            Assert.Null(exception);
+            await Task.Yield();
+        }
+
+        await cts.CancelAsync();
+        await writer.WaitAsync(TestCancellationToken);
+    }
+
+    [Fact]
+    public void FindHit_ReturnsMatchingHit()
+    {
+        var job = CreateJob();
+        var hit = CreateHit(1);
+        job.Hits.Add(hit);
+
+        var found = job.FindHit(hit.Id);
+
+        Assert.Same(hit, found);
+    }
+
     private static MultiRunJob CreateJob()
         => new(CreateSettingsService(), CreatePluginRepository());
 
@@ -179,6 +228,27 @@ private static async Task WaitUntilIdleAsync(MultiRunJob job)
         Assert.Equal(JobStatus.Idle, job.Status);
     }
 
+    private static global::RuriLib.Models.Hits.Hit CreateHit(int index)
+    {
+        const string wordlistTypeName = "default";
+
+        return new global::RuriLib.Models.Hits.Hit
+        {
+            Data = new DataLine(
+                $"user{index}:pass{index}",
+                new global::RuriLib.Models.Environment.WordlistType { Name = wordlistTypeName }),
+            CapturedData = new Dictionary<string, object> { ["token"] = $"abc{index}" },
+            Date = DateTime.UtcNow,
+            Type = "SUCCESS",
+            Config = new Config
+            {
+                Id = $"cfg-{index}",
+                Metadata = new ConfigMetadata { Name = "Config", Category = "Cat" }
+            },
+            DataPool = new TestDataPool([$"user{index}:pass{index}"], wordlistTypeName)
+        };
+    }
+
     private sealed class TestDataPool : DataPool
     {
         public TestDataPool()

RuriLib/Models/Jobs/MultiRunJob.cs

@@ -92,6 +92,7 @@ public class MultiRunJob : Job
 
     // Private fields
     private readonly string[] badStatuses = ["FAIL", "RETRY", "BAN", "ERROR", "INVALID"];
+    private readonly object hitsLock = new();
     private Parallelizer<MultiRunInput, CheckResult>? parallelizer;
     private ProxyPool? proxyPool;
     private Timer? tickTimer;
@@ -108,6 +109,28 @@ public class MultiRunJob : Job
     /// <summary>Gets the hits collected during the current run.</summary>
     public List<Hit> Hits { get; private set; } = [];
 
+    /// <summary>
+    /// Gets a snapshot of the hits collected during the current run.
+    /// </summary>
+    public List<Hit> GetHitsSnapshot()
+    {
+        lock (hitsLock)
+        {
+            return [.. Hits];
+        }
+    }
+
+    /// <summary>
+    /// Finds a hit by id in a thread-safe manner.
+    /// </summary>
+    public Hit? FindHit(string id)
+    {
+        lock (hitsLock)
+        {
+            return Hits.Find(h => h.Id == id);
+        }
+    }
+
     // Events
     /// <summary>Raised when a worker task fails.</summary>
     public event EventHandler<ErrorDetails<MultiRunInput>>? OnTaskError;
@@ -1004,7 +1027,11 @@ private void ResetStats()
         dataToCheck = 0;
         dataInvalid = 0;
         dataErrors = 0;
-        Hits = new();
+
+        lock (hitsLock)
+        {
+            Hits = [];
+        }
     }
 
     private void StatusChanged(object? sender, ParallelizerStatus status)
@@ -1075,7 +1102,11 @@ private async Task RegisterHit(CheckResult result)
         };
 
         // Add it to the local list of hits
-        Hits.Add(hit);
+        lock (hitsLock)
+        {
+            Hits.Add(hit);
+        }
+
         OnHit?.Invoke(this, hit);
 
         foreach (var hitOutput in HitOutputs)