fix(backend): Atomically mark remote account deletions

Signed-off-by: eternal-flame-AD <yume@yumechi.jp>

packages/backend/migration/1732071810971-IndexUserDeleted.js

@@ -0,0 +1,16 @@
+/*
+ * SPDX-FileCopyrightText: syuilo and misskey-project and yumechi
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+export class IndexUserDeleted1732071810971 {
+    name = 'IndexUserDeleted1732071810971'
+
+    async up(queryRunner) {
+        await queryRunner.query(`CREATE INDEX IF NOT EXISTS "IDX_199b79e682bdc5ba946f491686" ON "user" ("isDeleted")`);
+    }
+
+    async down(queryRunner) {
+        await queryRunner.query(`DROP INDEX IF EXISTS "IDX_199b79e682bdc5ba946f491686"`);
+    }
+}

packages/backend/src/core/DeleteAccountService.ts

@@ -47,6 +47,10 @@ export class DeleteAccountService {
 			});
 		}
 
+		if (!(await this.usersRepository.update({ id: user.id, isDeleted: false }, { isDeleted: true })).affected) {
+			return;
+		}
+
 		// 物理削除する前にDelete activityを送信する
 		if (this.userEntityService.isLocalUser(user)) {
 			// 知り得る全SharedInboxにDelete配信

packages/backend/src/core/RoleService.ts

@@ -488,6 +488,7 @@ export class RoleService implements OnApplicationShutdown, OnModuleInit {
 		return ids.length > 0
 			? await this.usersRepository.findBy({
 				id: In(ids),
+				isDeleted: false,
 			})
 			: [];
 	}

packages/backend/src/core/activitypub/ApInboxService.ts

@@ -509,19 +509,12 @@ export class ApInboxService {
 			return `skip: delete actor ${actor.uri} !== ${uri}`;
 		}
 
-		const user = await this.usersRepository.findOneBy({ id: actor.id });
+		if (!(await this.usersRepository.update({ id: actor.id, isDeleted: false }, { isDeleted: true })).affected) {
-		if (user == null) {
-			return 'skip: actor not found';
-		} else if (user.isDeleted) {
 			return 'skip: already deleted';
 		}
 
 		const job = await this.queueService.createDeleteAccountJob(actor);
 
-		await this.usersRepository.update(actor.id, {
-			isDeleted: true,
-		});
-
 		this.globalEventService.publishInternalEvent('remoteUserUpdated', { id: actor.id });
 
 		return `ok: queued ${job.name} ${job.id}`;

packages/backend/src/core/activitypub/models/ApPersonService.ts

@@ -557,7 +557,9 @@ export class ApPersonService implements OnModuleInit {
 		if (moving) updates.movedAt = new Date();
 
 		// Update user
-		await this.usersRepository.update(exist.id, updates);
+		if (!(await this.usersRepository.update({ id: exist.id, isDeleted: false }, updates)).affected) {
+			return 'skip';
+		}
 
 		if (person.publicKey) {
 			await this.userPublickeysRepository.update({ userId: exist.id }, {
@@ -662,7 +664,7 @@ export class ApPersonService implements OnModuleInit {
 
 	@bindThis
 	public async updateFeatured(userId: MiUser['id'], resolver?: Resolver): Promise<void> {
-		const user = await this.usersRepository.findOneByOrFail({ id: userId });
+		const user = await this.usersRepository.findOneByOrFail({ id: userId, isDeleted: false });
 		if (!this.userEntityService.isRemoteUser(user)) return;
 		if (!user.featured) return;
 

packages/backend/src/queue/processors/DeleteAccountProcessorService.ts

@@ -4,9 +4,9 @@
  */
 
 import { Inject, Injectable } from '@nestjs/common';
-import { MoreThan } from 'typeorm';
+import { DataSource, MoreThan, QueryFailedError, TypeORMError } from 'typeorm';
 import { DI } from '@/di-symbols.js';
-import type { DriveFilesRepository, NotesRepository, UserProfilesRepository, UsersRepository } from '@/models/_.js';
+import { MiUser, type DriveFilesRepository, type NotesRepository, type UserProfilesRepository, type UsersRepository } from '@/models/_.js';
 import type Logger from '@/logger.js';
 import { DriveService } from '@/core/DriveService.js';
 import type { MiDriveFile } from '@/models/DriveFile.js';
@@ -26,6 +26,9 @@ export class DeleteAccountProcessorService {
 		@Inject(DI.usersRepository)
 		private usersRepository: UsersRepository,
 
+		@Inject(DI.db)
+		private db: DataSource,
+
 		@Inject(DI.userProfilesRepository)
 		private userProfilesRepository: UserProfilesRepository,
 
@@ -52,6 +55,14 @@ export class DeleteAccountProcessorService {
 			return;
 		}
 
+		if (!user.isDeleted) {
+			this.logger.warn('User is not pre-marked as deleted, this is likely a bug');
+			if (process.env.NODE_ENV !== 'production') {
+				throw new Error('User is not pre-marked as deleted'); // make some noise to make sure tests fail
+			}
+			await this.usersRepository.update({ id: user.id }, { isDeleted: true });
+		}
+
 		{ // Delete notes
 			let cursor: MiNote['id'] | null = null;
 
@@ -121,13 +132,46 @@ export class DeleteAccountProcessorService {
 			}
 		}
 
-		// soft指定されている場合は物理削除しない
+		// Deadlockが発生した場合にリトライする
-		if (job.data.soft) {
+		for (let remaining = 3; remaining > 0; remaining--) {
-		// nop
+			try {
-		} else {
+				// soft指定されている場合は物理削除しない
-			await this.usersRepository.delete(job.data.user.id);
+				await this.db.transaction(async txn => {
+					// soft指定してもデータをすべで削除する
+					await txn.delete(MiUser, user.id);
+					if (job.data.soft) {
+						await txn.insert(MiUser, {
+							...user,
+							isRoot: false,
+							updatedAt: new Date(),
+							emojis: [],
+							hideOnlineStatus: true,
+							followersCount: 0,
+							followingCount: 0,
+							avatarUrl: null,
+							avatarId: null,
+							notesCount: 0,
+							inbox: null,
+							sharedInbox: null,
+							featured: null,
+							uri: null,
+							followersUri: null,
+							token: null,
+							isDeleted: true,
+						});
+					}
+				});
+				return 'Account deleted';
+			} catch (e) {
+				// 40P01 = deadlock_detected
+				// https://www.postgresql.org/docs/current/errcodes-appendix.html
+				if (remaining > 0 && e instanceof QueryFailedError && e.driverError.code === '40P01') {
+					this.logger.warn(`Deadlock occurred, retrying after 1s... [${remaining - 1} remaining]`);
+					await new Promise(resolve => setTimeout(resolve, 1000));
+					continue;
+				}
+				throw e;
+			}
 		}
-
-		return 'Account deleted';
 	}
 }

packages/backend/src/server/api/endpoints/users/show.ts

@@ -106,6 +106,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 					id: In(ps.userIds),
 				} : {
 					id: In(ps.userIds),
+					isDeleted: false,
 					isSuspended: false,
 				});