From 438ecfb41be09a9ea7aa3177a0e48468ea77cab0 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Tue, 12 Sep 2023 18:28:46 +0500 Subject: [PATCH] Force enumeration of global collections before using them (#106) This PR closes #105 This PR fixes exceptions caused by changing a collection's contents while it is being enumerated. This can often happen with Guild- and MemberDatas. By using `ToArray()` on these global collections and using it in the `foreach` loop, we create a new copy of the collection, preventing any modification to it. While this does introduce a lot of memory allocations, there is no fixing that. Usually, the fix to these exceptions would be to convert the `foreach` to a reverse-`for`. However, because indices cannot be used on these collections, that is not possible. Signed-off-by: Octol1ttle --- src/Commands/RemindCommandGroup.cs | 5 ++--- src/Services/GuildDataService.cs | 6 ++++-- src/Services/Update/MemberUpdateService.cs | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Commands/RemindCommandGroup.cs b/src/Commands/RemindCommandGroup.cs index 263d1ba..d437b5d 100644 --- a/src/Commands/RemindCommandGroup.cs +++ b/src/Commands/RemindCommandGroup.cs @@ -85,7 +85,7 @@ public class RemindCommandGroup : CommandGroup } var builder = new StringBuilder(); - for (var i = 0; i < data.Reminders.Count; i++) + for (var i = data.Reminders.Count - 1; i >= 0; i--) { var reminder = data.Reminders[i]; builder.AppendLine( @@ -168,8 +168,7 @@ public class RemindCommandGroup : CommandGroup [RequireContext(ChannelContext.Guild)] [UsedImplicitly] public async Task ExecuteDeleteReminderAsync( - [Description("Index of reminder to delete")] - [MinValue(0)] + [Description("Index of reminder to delete")] [MinValue(0)] int index) { if (!_context.TryGetContextIDs(out var guildId, out _, out var userId)) diff --git a/src/Services/GuildDataService.cs b/src/Services/GuildDataService.cs index 796b598..296377a 100644 --- a/src/Services/GuildDataService.cs +++ b/src/Services/GuildDataService.cs @@ -46,7 +46,8 @@ public sealed class GuildDataService : IHostedService { _logger.LogInformation("Saving guild data..."); var tasks = new List(); - foreach (var data in _datas.Values) + var datas = _datas.Values.ToArray(); + foreach (var data in datas) { await using var settingsStream = File.Create(data.SettingsPath); tasks.Add(JsonSerializer.SerializeAsync(settingsStream, data.Settings, cancellationToken: ct)); @@ -54,7 +55,8 @@ public sealed class GuildDataService : IHostedService await using var eventsStream = File.Create(data.ScheduledEventsPath); tasks.Add(JsonSerializer.SerializeAsync(eventsStream, data.ScheduledEvents, cancellationToken: ct)); - foreach (var memberData in data.MemberData.Values) + var memberDatas = data.MemberData.Values.ToArray(); + foreach (var memberData in memberDatas) { await using var memberDataStream = File.Create($"{data.MemberDataPath}/{memberData.Id}.json"); tasks.Add(JsonSerializer.SerializeAsync(memberDataStream, memberData, cancellationToken: ct)); diff --git a/src/Services/Update/MemberUpdateService.cs b/src/Services/Update/MemberUpdateService.cs index 472f844..631cc50 100644 --- a/src/Services/Update/MemberUpdateService.cs +++ b/src/Services/Update/MemberUpdateService.cs @@ -63,7 +63,8 @@ public sealed partial class MemberUpdateService : BackgroundService var guildData = await _guildData.GetData(guildId, ct); var defaultRole = GuildSettings.DefaultRole.Get(guildData.Settings); var failedResults = new List(); - foreach (var data in guildData.MemberData.Values) + var memberDatas = guildData.MemberData.Values.ToArray(); + foreach (var data in memberDatas) { var tickResult = await TickMemberDataAsync(guildId, guildData, defaultRole, data, ct); failedResults.AddIfFailed(tickResult);