From c302a5c2d7696bc9dddeabe914b92ad2fdc0b0ba Mon Sep 17 00:00:00 2001 From: Hazelnoot Date: Mon, 5 May 2025 17:44:00 -0400 Subject: [PATCH] reorder relay activities to avoid delivery race condition --- packages/backend/src/core/AccountMoveService.ts | 3 ++- .../backend/src/core/AccountUpdateService.ts | 9 +++------ packages/backend/src/core/NoteCreateService.ts | 14 +++++++------- packages/backend/src/core/NoteDeleteService.ts | 6 +++--- packages/backend/src/core/NoteEditService.ts | 14 +++++++------- packages/backend/src/core/NotePiningService.ts | 17 +++++++---------- packages/backend/src/core/PollService.ts | 9 +++------ .../core/activitypub/models/ApNoteService.ts | 4 ++-- packages/backend/src/misc/promise-tracker.ts | 4 ++++ .../src/server/api/endpoints/i/update.ts | 2 +- .../server/api/endpoints/notes/polls/vote.ts | 2 +- 11 files changed, 40 insertions(+), 44 deletions(-) diff --git a/packages/backend/src/core/AccountMoveService.ts b/packages/backend/src/core/AccountMoveService.ts index 7bf33e13c5..738026f753 100644 --- a/packages/backend/src/core/AccountMoveService.ts +++ b/packages/backend/src/core/AccountMoveService.ts @@ -95,11 +95,12 @@ export class AccountMoveService { const srcPerson = await this.apRendererService.renderPerson(src); const updateAct = this.apRendererService.addContext(this.apRendererService.renderUpdate(srcPerson, src)); await this.apDeliverManagerService.deliverToFollowers(src, updateAct); - this.relayService.deliverToRelays(src, updateAct); + await this.relayService.deliverToRelays(src, updateAct); // Deliver Move activity to the followers of the old account const moveAct = this.apRendererService.addContext(this.apRendererService.renderMove(src, dst)); await this.apDeliverManagerService.deliverToFollowers(src, moveAct); + await this.relayService.deliverToRelays(src, moveAct); // Publish meUpdated event const iObj = await this.userEntityService.pack(src.id, src, { schema: 'MeDetailed', includeSecrets: true }); diff --git a/packages/backend/src/core/AccountUpdateService.ts b/packages/backend/src/core/AccountUpdateService.ts index 69a57b4854..ef2962fc12 100644 --- a/packages/backend/src/core/AccountUpdateService.ts +++ b/packages/backend/src/core/AccountUpdateService.ts @@ -27,15 +27,12 @@ export class AccountUpdateService { } @bindThis - public async publishToFollowers(userId: MiUser['id']) { - const user = await this.usersRepository.findOneBy({ id: userId }); - if (user == null) throw new Error('user not found'); - + public async publishToFollowers(user: MiUser) { // フォロワーがリモートユーザーかつ投稿者がローカルユーザーならUpdateを配信 if (this.userEntityService.isLocalUser(user)) { const content = this.apRendererService.addContext(this.apRendererService.renderUpdate(await this.apRendererService.renderPerson(user), user)); - this.apDeliverManagerService.deliverToFollowers(user, content); - this.relayService.deliverToRelays(user, content); + await this.apDeliverManagerService.deliverToFollowers(user, content); + await this.relayService.deliverToRelays(user, content); } } } diff --git a/packages/backend/src/core/NoteCreateService.ts b/packages/backend/src/core/NoteCreateService.ts index fd6300483f..ed97908f66 100644 --- a/packages/backend/src/core/NoteCreateService.ts +++ b/packages/backend/src/core/NoteCreateService.ts @@ -51,7 +51,7 @@ import { FanoutTimelineService } from '@/core/FanoutTimelineService.js'; import { UtilityService } from '@/core/UtilityService.js'; import { UserBlockingService } from '@/core/UserBlockingService.js'; import { isReply } from '@/misc/is-reply.js'; -import { trackPromise } from '@/misc/promise-tracker.js'; +import { trackTask } from '@/misc/promise-tracker.js'; import { isUserRelated } from '@/misc/is-user-related.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { LatestNoteService } from '@/core/LatestNoteService.js'; @@ -729,7 +729,7 @@ export class NoteCreateService implements OnApplicationShutdown { //#region AP deliver if (!data.localOnly && this.userEntityService.isLocalUser(user)) { - (async () => { + trackTask(async () => { const noteActivity = await this.renderNoteOrRenoteActivity(data, note, user); const dm = this.apDeliverManagerService.createDeliverManager(user, noteActivity); @@ -755,12 +755,12 @@ export class NoteCreateService implements OnApplicationShutdown { dm.addFollowersRecipe(); } - if (['public'].includes(note.visibility)) { - this.relayService.deliverToRelays(user, noteActivity); - } + await dm.execute(); - trackPromise(dm.execute()); - })(); + if (['public'].includes(note.visibility)) { + await this.relayService.deliverToRelays(user, noteActivity); + } + }); } //#endregion } diff --git a/packages/backend/src/core/NoteDeleteService.ts b/packages/backend/src/core/NoteDeleteService.ts index 8ec05c88dc..9b6c4754d1 100644 --- a/packages/backend/src/core/NoteDeleteService.ts +++ b/packages/backend/src/core/NoteDeleteService.ts @@ -247,11 +247,11 @@ export class NoteDeleteService { @bindThis private async deliverToConcerned(user: { id: MiLocalUser['id']; host: null; }, note: MiNote, content: any) { - this.apDeliverManagerService.deliverToFollowers(user, content); - this.relayService.deliverToRelays(user, content); - this.apDeliverManagerService.deliverToUsers(user, content, [ + await this.apDeliverManagerService.deliverToFollowers(user, content); + await this.apDeliverManagerService.deliverToUsers(user, content, [ ...await this.getMentionedRemoteUsers(note), ...await this.getRenotedOrRepliedRemoteUsers(note), ]); + await this.relayService.deliverToRelays(user, content); } } diff --git a/packages/backend/src/core/NoteEditService.ts b/packages/backend/src/core/NoteEditService.ts index e70ecf396d..332560154d 100644 --- a/packages/backend/src/core/NoteEditService.ts +++ b/packages/backend/src/core/NoteEditService.ts @@ -46,7 +46,7 @@ import { UtilityService } from '@/core/UtilityService.js'; import { UserBlockingService } from '@/core/UserBlockingService.js'; import { CacheService } from '@/core/CacheService.js'; import { isReply } from '@/misc/is-reply.js'; -import { trackPromise } from '@/misc/promise-tracker.js'; +import { trackTask } from '@/misc/promise-tracker.js'; import { isUserRelated } from '@/misc/is-user-related.js'; import { IdentifiableError } from '@/misc/identifiable-error.js'; import { LatestNoteService } from '@/core/LatestNoteService.js'; @@ -669,7 +669,7 @@ export class NoteEditService implements OnApplicationShutdown { //#region AP deliver if (!data.localOnly && this.userEntityService.isLocalUser(user)) { - (async () => { + trackTask(async () => { const noteActivity = await this.renderNoteOrRenoteActivity(data, note, user); const dm = this.apDeliverManagerService.createDeliverManager(user, noteActivity); @@ -713,12 +713,12 @@ export class NoteEditService implements OnApplicationShutdown { } } - if (['public'].includes(note.visibility)) { - this.relayService.deliverToRelays(user, noteActivity); - } + await dm.execute(); - trackPromise(dm.execute()); - })(); + if (['public'].includes(note.visibility)) { + await this.relayService.deliverToRelays(user, noteActivity); + } + }); } //#endregion } diff --git a/packages/backend/src/core/NotePiningService.ts b/packages/backend/src/core/NotePiningService.ts index d38b48b65d..6ab7268254 100644 --- a/packages/backend/src/core/NotePiningService.ts +++ b/packages/backend/src/core/NotePiningService.ts @@ -49,7 +49,7 @@ export class NotePiningService { * @param noteId */ @bindThis - public async addPinned(user: { id: MiUser['id']; host: MiUser['host']; }, noteId: MiNote['id']) { + public async addPinned(user: MiUser, noteId: MiNote['id']) { // Fetch pinee const note = await this.notesRepository.findOneBy({ id: noteId, @@ -78,7 +78,7 @@ export class NotePiningService { // Deliver to remote followers if (this.userEntityService.isLocalUser(user) && !note.localOnly && ['public', 'home'].includes(note.visibility)) { - this.deliverPinnedChange(user.id, note.id, true); + this.deliverPinnedChange(user, note.id, true); } } @@ -88,7 +88,7 @@ export class NotePiningService { * @param noteId */ @bindThis - public async removePinned(user: { id: MiUser['id']; host: MiUser['host']; }, noteId: MiNote['id']) { + public async removePinned(user: MiUser, noteId: MiNote['id']) { // Fetch unpinee const note = await this.notesRepository.findOneBy({ id: noteId, @@ -106,22 +106,19 @@ export class NotePiningService { // Deliver to remote followers if (this.userEntityService.isLocalUser(user) && !note.localOnly && ['public', 'home'].includes(note.visibility)) { - this.deliverPinnedChange(user.id, noteId, false); + this.deliverPinnedChange(user, noteId, false); } } @bindThis - public async deliverPinnedChange(userId: MiUser['id'], noteId: MiNote['id'], isAddition: boolean) { - const user = await this.usersRepository.findOneBy({ id: userId }); - if (user == null) throw new Error('user not found'); - + public async deliverPinnedChange(user: MiUser, noteId: MiNote['id'], isAddition: boolean) { if (!this.userEntityService.isLocalUser(user)) return; const target = `${this.config.url}/users/${user.id}/collections/featured`; const item = `${this.config.url}/notes/${noteId}`; const content = this.apRendererService.addContext(isAddition ? this.apRendererService.renderAdd(user, target, item) : this.apRendererService.renderRemove(user, target, item)); - this.apDeliverManagerService.deliverToFollowers(user, content); - this.relayService.deliverToRelays(user, content); + await this.apDeliverManagerService.deliverToFollowers(user, content); + await this.relayService.deliverToRelays(user, content); } } diff --git a/packages/backend/src/core/PollService.ts b/packages/backend/src/core/PollService.ts index d6364613bd..33262a4804 100644 --- a/packages/backend/src/core/PollService.ts +++ b/packages/backend/src/core/PollService.ts @@ -90,10 +90,7 @@ export class PollService { } @bindThis - public async deliverQuestionUpdate(noteId: MiNote['id']) { - const note = await this.notesRepository.findOneBy({ id: noteId }); - if (note == null) throw new Error('note not found'); - + public async deliverQuestionUpdate(note: MiNote) { if (note.localOnly) return; const user = await this.usersRepository.findOneBy({ id: note.userId }); @@ -101,8 +98,8 @@ export class PollService { if (this.userEntityService.isLocalUser(user)) { const content = this.apRendererService.addContext(this.apRendererService.renderUpdate(await this.apRendererService.renderNote(note, user, false), user)); - this.apDeliverManagerService.deliverToFollowers(user, content); - this.relayService.deliverToRelays(user, content); + await this.apDeliverManagerService.deliverToFollowers(user, content); + await this.relayService.deliverToRelays(user, content); } } } diff --git a/packages/backend/src/core/activitypub/models/ApNoteService.ts b/packages/backend/src/core/activitypub/models/ApNoteService.ts index 11f5bbd943..f6152e3888 100644 --- a/packages/backend/src/core/activitypub/models/ApNoteService.ts +++ b/packages/backend/src/core/activitypub/models/ApNoteService.ts @@ -296,7 +296,7 @@ export class ApNoteService { await this.pollService.vote(actor, reply, index); // リモートフォロワーにUpdate配信 - this.pollService.deliverQuestionUpdate(reply.id); + this.pollService.deliverQuestionUpdate(reply); } return null; }; @@ -493,7 +493,7 @@ export class ApNoteService { await this.pollService.vote(actor, reply, index); // リモートフォロワーにUpdate配信 - this.pollService.deliverQuestionUpdate(reply.id); + this.pollService.deliverQuestionUpdate(reply); } return null; }; diff --git a/packages/backend/src/misc/promise-tracker.ts b/packages/backend/src/misc/promise-tracker.ts index 8a52ca703e..76b4dd810c 100644 --- a/packages/backend/src/misc/promise-tracker.ts +++ b/packages/backend/src/misc/promise-tracker.ts @@ -5,6 +5,10 @@ const promiseRefs: Set>> = new Set(); +export function trackTask(task: () => Promise): void { + trackPromise(task()); +} + /** * This tracks promises that other modules decided not to wait for, * and makes sure they are all settled before fully closing down the server. diff --git a/packages/backend/src/server/api/endpoints/i/update.ts b/packages/backend/src/server/api/endpoints/i/update.ts index 094c3da8e6..5f93597fd7 100644 --- a/packages/backend/src/server/api/endpoints/i/update.ts +++ b/packages/backend/src/server/api/endpoints/i/update.ts @@ -614,7 +614,7 @@ export default class extends Endpoint { // eslint- // フォロワーにUpdateを配信 if (this.userNeedsPublishing(user, updates) || this.profileNeedsPublishing(profile, updatedProfile)) { - this.accountUpdateService.publishToFollowers(user.id); + this.accountUpdateService.publishToFollowers(user); } return iObj; diff --git a/packages/backend/src/server/api/endpoints/notes/polls/vote.ts b/packages/backend/src/server/api/endpoints/notes/polls/vote.ts index a5014a490f..0b318304f3 100644 --- a/packages/backend/src/server/api/endpoints/notes/polls/vote.ts +++ b/packages/backend/src/server/api/endpoints/notes/polls/vote.ts @@ -174,7 +174,7 @@ export default class extends Endpoint { // eslint- } // リモートフォロワーにUpdate配信 - this.pollService.deliverQuestionUpdate(note.id); + this.pollService.deliverQuestionUpdate(note); }); } }