From 1a33fc2573b568cea1a57bebdb0289a3879ac62b Mon Sep 17 00:00:00 2001 From: Sucukdeluxe Date: Wed, 17 Jun 2026 04:57:23 +0200 Subject: [PATCH] Fix: Einstellungen gehen nicht mehr durch zeitgleiches Speichern verloren (Lost Update) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Der asynchrone Settings-Writer hatte — anders als der Session-Writer — keinen Generations-Schutz. Lief eine periodische async Settings-Speicherung gerade, während gleichzeitig synchron saveSettings() lief (z.B. Nutzer ändert eine Option, oder ein Settings-Backup wird wiederhergestellt), konnte der async rename die frisch synchron geschriebene Datei wieder mit dem ALTEN Stand überschreiben. Die gerade gespeicherte Änderung war damit auf der Platte (und im .bak) verloren — bis zur nächsten Speicherung. Fix: Der Settings-Pfad spiegelt jetzt exakt den bereits abgesicherten Session-Pfad: - Eigener Generations-Zähler syncSettingsSaveGeneration; saveSettings() (sync) erhöht ihn. - writeSettingsPayload bekommt die zum Zeitpunkt des Aufrufs erfasste Generation und bricht vor rename UND vor dem EXDEV-Copy ab, wenn inzwischen eine synchrone Speicherung passiert ist (generation < aktuell). - saveSettingsAsync/saveSettingsPayloadAsync trägt die ORIGINAL-Generation auch durch die Warteschlange (vorher wurde beim Abarbeiten der Queue eine frische Generation erfasst → hätte den Schutz ausgehebelt). - Eigener Zähler statt Wiederverwendung von syncSaveGeneration, damit eine synchrone Settings-Speicherung keine laufenden async SESSION-Schreibvorgänge fälschlich verwirft (keine Datei-übergreifende Kopplung). Zusätzlich (verwandt): shutdown() ruft jetzt cancelPendingAsyncSaves() vor der finalen synchronen Speicherung auf, damit eine noch laufende/eingereihte async Settings-Schreibung den Shutdown-Save nicht mehr überholen kann. Test: in-flight + eingereihte async Settings-Speicherung (ALT), dann synchron NEU — nach dem Settle steht NEU auf der Platte. Ohne den Generations-Schutz gewinnt ALT (rot bewiesen). --- src/main/app-controller.ts | 1 + src/main/storage.ts | 32 ++++++++++++++++++++++-------- tests/session-restart-loss.test.ts | 28 +++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/main/app-controller.ts b/src/main/app-controller.ts index 9a402a6..c3fc94e 100644 --- a/src/main/app-controller.ts +++ b/src/main/app-controller.ts @@ -789,6 +789,7 @@ public async checkDebridAccounts(): Promise { } stopDebugServer(); abortActiveUpdateDownload(); + cancelPendingAsyncSaves(); this.manager.prepareForShutdown(); this.megaWebFallback.dispose(); this.realDebridWebFallback.dispose(); diff --git a/src/main/storage.ts b/src/main/storage.ts index 56602f2..ae0204f 100644 --- a/src/main/storage.ts +++ b/src/main/storage.ts @@ -897,6 +897,7 @@ function readSessionFile(filePath: string): SessionState | null { } export function saveSettings(paths: StoragePaths, settings: AppSettings): void { + syncSettingsSaveGeneration += 1; ensureBaseDir(paths.baseDir); if (fs.existsSync(paths.configFile)) { try { @@ -917,17 +918,26 @@ export function saveSettings(paths: StoragePaths, settings: AppSettings): void { } let asyncSettingsSaveRunning = false; -let asyncSettingsSaveQueued: { paths: StoragePaths; settings: AppSettings } | null = null; +let asyncSettingsSaveQueued: { paths: StoragePaths; payload: string; generation: number } | null = null; +let syncSettingsSaveGeneration = 0; -async function writeSettingsPayload(paths: StoragePaths, payload: string): Promise { +async function writeSettingsPayload(paths: StoragePaths, payload: string, generation: number): Promise { await fs.promises.mkdir(paths.baseDir, { recursive: true }); await fsp.copyFile(paths.configFile, `${paths.configFile}.bak`).catch(() => {}); const tempPath = `${paths.configFile}.settings.tmp`; await fsp.writeFile(tempPath, payload, "utf8"); + if (generation < syncSettingsSaveGeneration) { + await fsp.rm(tempPath, { force: true }).catch(() => {}); + return; + } try { await fsp.rename(tempPath, paths.configFile); } catch (renameError: unknown) { if (renameError && typeof renameError === "object" && "code" in renameError && (renameError as NodeJS.ErrnoException).code === "EXDEV") { + if (generation < syncSettingsSaveGeneration) { + await fsp.rm(tempPath, { force: true }).catch(() => {}); + return; + } await fsp.copyFile(tempPath, paths.configFile); await fsp.rm(tempPath, { force: true }).catch(() => {}); } else { @@ -937,16 +947,14 @@ async function writeSettingsPayload(paths: StoragePaths, payload: string): Promi } } -export async function saveSettingsAsync(paths: StoragePaths, settings: AppSettings): Promise { - const persisted = sanitizeCredentialPersistence(normalizeSettings(settings)); - const payload = JSON.stringify(persisted, safeJsonReplacer, 2); +async function saveSettingsPayloadAsync(paths: StoragePaths, payload: string, generation: number): Promise { if (asyncSettingsSaveRunning) { - asyncSettingsSaveQueued = { paths, settings }; + asyncSettingsSaveQueued = { paths, payload, generation }; return; } asyncSettingsSaveRunning = true; try { - await writeSettingsPayload(paths, payload); + await writeSettingsPayload(paths, payload, generation); } catch (error) { logger.error(`Async Settings-Save fehlgeschlagen: ${String(error)}`); } finally { @@ -954,11 +962,18 @@ export async function saveSettingsAsync(paths: StoragePaths, settings: AppSettin if (asyncSettingsSaveQueued) { const queued = asyncSettingsSaveQueued; asyncSettingsSaveQueued = null; - void saveSettingsAsync(queued.paths, queued.settings); + void saveSettingsPayloadAsync(queued.paths, queued.payload, queued.generation); } } } +export async function saveSettingsAsync(paths: StoragePaths, settings: AppSettings): Promise { + const generation = syncSettingsSaveGeneration; + const persisted = sanitizeCredentialPersistence(normalizeSettings(settings)); + const payload = JSON.stringify(persisted, safeJsonReplacer, 2); + await saveSettingsPayloadAsync(paths, payload, generation); +} + export function emptySession(): SessionState { return { version: 2, @@ -1122,6 +1137,7 @@ export function cancelPendingAsyncSaves(): void { asyncSaveQueued = null; asyncSettingsSaveQueued = null; syncSaveGeneration += 1; + syncSettingsSaveGeneration += 1; } export async function saveSessionAsync(paths: StoragePaths, session: SessionState): Promise { diff --git a/tests/session-restart-loss.test.ts b/tests/session-restart-loss.test.ts index 6f2de73..73f4544 100644 --- a/tests/session-restart-loss.test.ts +++ b/tests/session-restart-loss.test.ts @@ -8,9 +8,13 @@ import { createStoragePaths, emptySession, loadSession, + loadSettings, saveSession, - saveSessionAsync + saveSessionAsync, + saveSettings, + saveSettingsAsync } from "../src/main/storage"; +import { defaultSettings } from "../src/main/constants"; const tempDirs: string[] = []; @@ -129,4 +133,26 @@ describe("session restart loss", () => { const loaded = loadSession(paths); expect(Object.keys(loaded.packages).sort()).toEqual(["A", "B"]); }); + + it("does not let an in-flight/queued async settings save clobber a newer synchronous saveSettings", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-settings-race-")); + tempDirs.push(dir); + const paths = createStoragePaths(dir); + + cancelPendingAsyncSaves(); + await settle(50); + + const withName = (name: string) => ({ ...defaultSettings(), packageName: name }); + + saveSettings(paths, withName("OLD")); + const inflight = saveSettingsAsync(paths, withName("OLD")); + const queued = saveSettingsAsync(paths, withName("OLD")); + saveSettings(paths, withName("NEW")); + + await inflight; + await queued; + await settle(); + + expect(loadSettings(paths).packageName).toBe("NEW"); + }); });