From f073018e955c4f91d343370798b6dc6b2907b15a Mon Sep 17 00:00:00 2001 From: Laura Hausmann Date: Sat, 17 Feb 2024 19:42:15 +0100 Subject: [PATCH] [backend/federation] Fix race condition when updating a user during a request --- .../Core/Database/Tables/DriveFile.cs | 4 ++-- .../ActivityPub/ActivityDeliverService.cs | 16 ++++++---------- .../Core/Federation/ActivityPub/UserResolver.cs | 7 ++++++- Iceshrimp.Backend/Core/Services/DriveService.cs | 5 +++++ Iceshrimp.Backend/Core/Services/UserService.cs | 13 ++++++++++--- 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/Iceshrimp.Backend/Core/Database/Tables/DriveFile.cs b/Iceshrimp.Backend/Core/Database/Tables/DriveFile.cs index 4ceccb73..6d11013c 100644 --- a/Iceshrimp.Backend/Core/Database/Tables/DriveFile.cs +++ b/Iceshrimp.Backend/Core/Database/Tables/DriveFile.cs @@ -26,7 +26,7 @@ public class DriveFile : IEntity /// The created date of the DriveFile. /// [Column("createdAt")] - public DateTime CreatedAt { get; set; } = DateTime.UtcNow; + public DateTime CreatedAt { get; set; } /// /// The owner ID. @@ -188,7 +188,7 @@ public class DriveFile : IEntity [Key] [Column("id")] [StringLength(32)] - public string Id { get; set; } = IdHelpers.GenerateSlowflakeId(); + public string Id { get; set; } = null!; public class FileProperties { diff --git a/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityDeliverService.cs b/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityDeliverService.cs index d2ecd13d..c90bac42 100644 --- a/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityDeliverService.cs +++ b/Iceshrimp.Backend/Core/Federation/ActivityPub/ActivityDeliverService.cs @@ -16,11 +16,9 @@ public class ActivityDeliverService(ILogger logger, Queu await queueService.PreDeliverQueue.EnqueueAsync(new PreDeliverJob { - ActorId = actor.Id, - RecipientIds = recipients.Select(p => p.Id).ToList(), - SerializedActivity = - JsonConvert.SerializeObject(activity, - LdHelpers.JsonSerializerSettings), + ActorId = actor.Id, + RecipientIds = recipients.Select(p => p.Id).ToList(), + SerializedActivity = JsonConvert.SerializeObject(activity, LdHelpers.JsonSerializerSettings), DeliverToFollowers = true }); } @@ -32,11 +30,9 @@ public class ActivityDeliverService(ILogger logger, Queu await queueService.PreDeliverQueue.EnqueueAsync(new PreDeliverJob { - ActorId = actor.Id, - RecipientIds = recipients.Select(p => p.Id).ToList(), - SerializedActivity = - JsonConvert.SerializeObject(activity, - LdHelpers.JsonSerializerSettings), + ActorId = actor.Id, + RecipientIds = recipients.Select(p => p.Id).ToList(), + SerializedActivity = JsonConvert.SerializeObject(activity, LdHelpers.JsonSerializerSettings), DeliverToFollowers = false }); } diff --git a/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs b/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs index 31637f8b..bd58a8af 100644 --- a/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs +++ b/Iceshrimp.Backend/Core/Federation/ActivityPub/UserResolver.cs @@ -112,13 +112,18 @@ public class UserResolver( private async Task GetUpdatedUser(User user) { if (!user.NeedsUpdate) return user; + user.LastFetchedAt = DateTime.UtcNow; // Prevent multiple background tasks from being started try { var task = followupTaskSvc.ExecuteTask("UpdateUserAsync", async provider => { + // Get a fresh UserService instance in a new scope var bgUserSvc = provider.GetRequiredService(); - await bgUserSvc.UpdateUserAsync(user); + + // Use the id overload so it doesn't attempt to insert in the main thread's DbContext + var fetchedUser = await bgUserSvc.UpdateUserAsync(user.Id); + user = fetchedUser; }); // Return early, but continue execution in background diff --git a/Iceshrimp.Backend/Core/Services/DriveService.cs b/Iceshrimp.Backend/Core/Services/DriveService.cs index a49dabbd..5f631920 100644 --- a/Iceshrimp.Backend/Core/Services/DriveService.cs +++ b/Iceshrimp.Backend/Core/Services/DriveService.cs @@ -4,6 +4,7 @@ using Iceshrimp.Backend.Core.Configuration; using Iceshrimp.Backend.Core.Database; using Iceshrimp.Backend.Core.Database.Tables; using Iceshrimp.Backend.Core.Federation.Cryptography; +using Iceshrimp.Backend.Core.Helpers; using Iceshrimp.Backend.Core.Middleware; using Iceshrimp.Backend.Core.Queues; using Microsoft.EntityFrameworkCore; @@ -240,6 +241,8 @@ public class DriveService( file = new DriveFile { + Id = IdHelpers.GenerateSlowflakeId(), + CreatedAt = DateTime.UtcNow, User = user, UserHost = user.Host, Sha256 = digest, @@ -332,6 +335,8 @@ file static class DriveFileExtensions return new DriveFile { + Id = IdHelpers.GenerateSlowflakeId(), + CreatedAt = DateTime.UtcNow, User = user, Blurhash = file.Blurhash, Type = file.Type, diff --git a/Iceshrimp.Backend/Core/Services/UserService.cs b/Iceshrimp.Backend/Core/Services/UserService.cs index f127e318..5e22f78f 100644 --- a/Iceshrimp.Backend/Core/Services/UserService.cs +++ b/Iceshrimp.Backend/Core/Services/UserService.cs @@ -129,7 +129,7 @@ public class UserService( try { await db.AddRangeAsync(user, profile, publicKey); - + // We need to do this after calling db.Add(Range) to ensure data consistency var processPendingDeletes = await ResolveAvatarAndBanner(user, actor); await db.SaveChangesAsync(); @@ -158,9 +158,16 @@ public class UserService( } } - public async Task UpdateUserAsync(User user, ASActor? actor = null) + public async Task UpdateUserAsync(string id) { - if (!user.NeedsUpdate && actor == null) return user; + var user = await db.Users.IncludeCommonProperties().FirstOrDefaultAsync(p => p.Id == id) ?? + throw new Exception("Cannot update nonexistent user"); + return await UpdateUserAsync(user, force: true); + } + + public async Task UpdateUserAsync(User user, ASActor? actor = null, bool force = false) + { + if (!user.NeedsUpdate && actor == null && !force) return user; if (actor is { IsUnresolved: true } or { Username: null }) actor = null; // This will trigger a fetch a couple lines down