From 1b959568d75d5c270547dad53fae1c005ff16c73 Mon Sep 17 00:00:00 2001 From: Edoardo Spadoni Date: Tue, 15 Sep 2026 21:02:29 +0200 Subject: [PATCH] fix: prevent user data corruption on unclean Windows shutdown NethLink could hang forever on the "starting application" splash screen after a power loss, with both user_data.json and available_user_data.json recovered at roughly the right size but filled with 0x00. Two independent causes: - saveToDisk() wrote straight to the destination with writeFileSync and never flushed. NTFS journals metadata only, so after a power loss the new file size is replayed from the journal while the data pages are still in the page cache and lost, leaving a NUL filled file. Writes now go to a temp file, get fsync'd and are renamed over the target, so the previous content stays intact until the new one is durable on disk, and a .bak of it is kept. - getAvailableFromDisk() parsed the file with no try/catch, so the SyntaxError propagated out of the fire and forget startApp(), where nothing caught it: showLogin() and the splash screen teardown were never reached. Reads now reject empty, NUL filled and unparsable files, fall back to the backup and never throw, startApp() is guarded, and unhandled rejections are logged. Reads consult the backup only when the primary file exists but is unusable, so removing the json files by hand still forces a clean login. Already affected installations recover on their own: they land on the login screen instead of a frozen splash, with no manual file renaming. --- src/main/lib/mainStore.ts | 178 +++++++++++++++++++++++++++++--------- src/main/main.ts | 62 +++++++++++-- 2 files changed, 193 insertions(+), 47 deletions(-) diff --git a/src/main/lib/mainStore.ts b/src/main/lib/mainStore.ts index a6626148..0ccfc215 100644 --- a/src/main/lib/mainStore.ts +++ b/src/main/lib/mainStore.ts @@ -1,4 +1,4 @@ -import { Account, LocalStorageData } from '@shared/types'; +import { Account, AuthAppData, LocalStorageData } from '@shared/types'; import { app, ipcMain } from 'electron'; import path from 'path'; import fs from 'fs'; @@ -8,6 +8,105 @@ import { difference } from 'lodash'; const AVAILABLE_USER_DATA_PATH = path.join(app.getPath("userData"), `available_user_data.json`); +// on Windows the rename can transiently fail while an antivirus keeps the target open +const RENAME_RETRY_DELAYS_MS = [10, 30, 90] + +function sleepSync(ms: number) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms) +} + +function renameWithRetry(from: string, to: string) { + for (let attempt = 0; ; attempt++) { + try { + fs.renameSync(from, to) + return + } catch (e: any) { + if (attempt >= RENAME_RETRY_DELAYS_MS.length) { + throw e + } + Log.warning('rename of', from, 'failed, retrying:', e?.code || e) + sleepSync(RENAME_RETRY_DELAYS_MS[attempt]) + } + } +} + +/** + * NTFS journals metadata only: after an unclean shutdown the new file size can be + * recovered from the journal while the data pages are still sitting in the page + * cache, leaving a file of the expected length filled with 0x00. Writing to a + * temporary file, flushing it to the platters and only then renaming it over the + * target keeps the previous content intact until the new one is durable on disk. + * The fsync is the load bearing part: without it the rename alone still allows a + * NUL-filled file to show up under the final name. + */ +function writeJsonAtomic(target: string, content: string) { + const tmp = `${target}.tmp` + const fd = fs.openSync(tmp, 'w') + try { + fs.writeFileSync(fd, content) + fs.fsyncSync(fd) + } finally { + fs.closeSync(fd) + } + try { + if (fs.existsSync(target)) { + fs.copyFileSync(target, `${target}.bak`) + } + } catch (e) { + Log.warning('unable to refresh the backup of', target, e) + } + renameWithRetry(tmp, target) +} + +function isAllZeroes(buffer: Buffer) { + for (let i = 0; i < buffer.length; i++) { + if (buffer[i] !== 0) return false + } + return true +} + +/** + * Returns undefined - never throws - when the file is missing, empty, recovered + * as NUL bytes after a crash, or simply not valid JSON. + */ +function parseJsonFile(filePath: string): any | undefined { + try { + if (!fs.existsSync(filePath)) return undefined + const buffer = fs.readFileSync(filePath) + if (buffer.length === 0) { + Log.warning('STORE', filePath, 'is empty') + return undefined + } + if (isAllZeroes(buffer)) { + Log.warning('STORE', filePath, 'contains only NUL bytes - corrupted by an unclean shutdown') + return undefined + } + return JSON.parse(buffer.toString('utf-8')) + } catch (e) { + Log.warning('STORE unable to read', filePath, e) + return undefined + } +} + +/** + * The backup is consulted only when the primary file is there but unusable: a file + * that is simply missing means a fresh install - or someone removing it on purpose to + * force a clean login - and bringing it back from the backup would defeat that. + */ +function readJsonSafe(filePath: string, fallback: T): T { + const data = parseJsonFile(filePath) + if (data !== undefined) return data as T + if (fs.existsSync(filePath)) { + const backup = parseJsonFile(`${filePath}.bak`) + if (backup !== undefined) { + Log.warning('STORE restored', filePath, 'from its backup') + return backup as T + } + } + Log.warning('STORE no usable data for', filePath, '- falling back to defaults') + return fallback +} + class Store { assignedInstanceID; USER_DATA_PATH @@ -21,23 +120,19 @@ class Store { } Log.info({ assignedInstanceID: this.assignedInstanceID }) this.USER_DATA_PATH = path.join(app.getPath("userData"), `user_data${this.assignedInstanceID || ''}.json`); - if (!fs.existsSync(this.USER_DATA_PATH) || !fs.existsSync(AVAILABLE_USER_DATA_PATH)) { - if (!fs.existsSync(AVAILABLE_USER_DATA_PATH)) { - if (!fs.existsSync(this.USER_DATA_PATH)) { - fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify({})) - } else { - const data = fs.readFileSync(this.USER_DATA_PATH, 'utf-8'); - const retrivedStore: LocalStorageData = JSON.parse(data); - if (retrivedStore.auth?.availableAccounts) { - fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify(retrivedStore.auth!.availableAccounts)) - } else { - fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify({})) - } - } + const hasUserData = fs.existsSync(this.USER_DATA_PATH) + const hasAvailableUserData = fs.existsSync(AVAILABLE_USER_DATA_PATH) + if (!hasUserData || !hasAvailableUserData) { + if (!hasAvailableUserData) { + // migration path for the installations where the accounts lived only in user_data.json + const userData = readJsonSafe>(this.USER_DATA_PATH, {}) + writeJsonAtomic(AVAILABLE_USER_DATA_PATH, JSON.stringify(userData?.auth?.availableAccounts ?? {})) + } + if (!hasUserData) { + writeJsonAtomic(this.USER_DATA_PATH, JSON.stringify({})) } - !fs.existsSync(this.USER_DATA_PATH) && fs.writeFileSync(this.USER_DATA_PATH, JSON.stringify({})) } else { - this.store = this.getFromDisk() + this.store = this.getFromDisk() ?? ({} as T) } } @@ -57,46 +152,51 @@ class Store { } } - updateStore(newState: T, from: string) { + updateStore(newState: T | null | undefined, from: string) { const diff = difference(Object.values(newState as any || {}), Object.values(this.store as any || {})) Log.debug('STORE update shared store from', from, Object.keys(newState as any || {})) if (diff.length > 0 || this.store === undefined) { - this.store = Object.assign({}, newState) + this.store = Object.assign({}, newState ?? ({} as T)) } } saveToDisk(forceSave: boolean = false) { - const availableUserData = (this.store as LocalStorageData).auth?.availableAccounts - fs.writeFileSync(this.USER_DATA_PATH, JSON.stringify(this.store)); + const availableUserData = (this.store as LocalStorageData)?.auth?.availableAccounts + try { + writeJsonAtomic(this.USER_DATA_PATH, JSON.stringify(this.store ?? {})); + } catch (e) { + Log.error('STORE unable to persist', this.USER_DATA_PATH, e) + } if (Object.keys(availableUserData || {}).length > 0 || forceSave) { - fs.writeFileSync(AVAILABLE_USER_DATA_PATH, JSON.stringify(availableUserData)); + try { + writeJsonAtomic(AVAILABLE_USER_DATA_PATH, JSON.stringify(availableUserData ?? {})); + } catch (e) { + Log.error('STORE unable to persist', AVAILABLE_USER_DATA_PATH, e) + } } } getAvailableFromDisk(): { [accountUID: string]: Account; } { - if (fs.existsSync(AVAILABLE_USER_DATA_PATH)) { - const availableUserData = fs.readFileSync(AVAILABLE_USER_DATA_PATH, 'utf-8'); - return JSON.parse(availableUserData) + const availableUserData = readJsonSafe<{ [accountUID: string]: Account } | null>(AVAILABLE_USER_DATA_PATH, null) + if (!availableUserData || typeof availableUserData !== 'object') { + return {} } - return {} + return availableUserData } - getFromDisk() { - try { - const data = fs.readFileSync(this.USER_DATA_PATH, 'utf-8'); - const availableUserData = fs.readFileSync(AVAILABLE_USER_DATA_PATH, 'utf-8'); - const retrivedStore = JSON.parse(data); - (retrivedStore as LocalStorageData).auth!.availableAccounts = JSON.parse(availableUserData) - return retrivedStore - } catch (error) { - Log.error('retrieving user data', error); - // you may want to propagate the error, up to you - return null; + getFromDisk(): T | null { + const data = readJsonSafe(this.USER_DATA_PATH, null) + if (!data || typeof data !== 'object') { + Log.error('retrieving user data: no usable content in', this.USER_DATA_PATH) + return null } + const retrivedStore = data as LocalStorageData + retrivedStore.auth = { + ...(retrivedStore.auth ?? {}), + availableAccounts: this.getAvailableFromDisk() + } as AuthAppData + return retrivedStore as T } } export const store: Store = new Store() - - - diff --git a/src/main/main.ts b/src/main/main.ts index b0ccaf87..6d57e23b 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -6,7 +6,7 @@ import { registerIpcEvents, isCallActive, disableCommandBarShortcuts } from '@/l import { AccountController } from './classes/controllers' import { PhoneIslandController } from './classes/controllers/PhoneIslandController' import { CommandBarController } from './classes/controllers/CommandBarController' -import { Account, AuthAppData, AvailableThemes } from '@shared/types' +import { Account, AuthAppData, AvailableThemes, LocalStorageData } from '@shared/types' import { TrayController } from './classes/controllers/TrayController' import { LoginController } from './classes/controllers/LoginController' import { join, resolve } from 'path' @@ -56,6 +56,7 @@ function startup() { app.setAppUserModelId('com.nethesis.nethlink.app') //must match electron-builder appId so Windows notification permissions and taskbar pinning work correctly ///LOGGER startLogger() + attachGlobalErrorHandlers() //windows const gotTheLock = multipleInstances || app.requestSingleInstanceLock() @@ -214,6 +215,21 @@ function startLogger() { deleteLogFile() } +/** + * Without these the main process swallows any rejection coming from the several fire + * and forget async flows, which makes issues that only show up on customer machines + * - a crash during startup leaving the app stuck on the splash screen - invisible in + * the logs. + */ +function attachGlobalErrorHandlers() { + process.on('unhandledRejection', (reason) => { + Log.error('UNHANDLED REJECTION', reason) + }) + process.on('uncaughtException', (error) => { + Log.error('UNCAUGHT EXCEPTION', error) + }) +} + function attachOnReadyProcess() { new AppController(app) new NetworkController() @@ -242,13 +258,13 @@ function attachOnReadyProcess() { // Wait for unlock-screen event before starting powerMonitor.once('unlock-screen', () => { Log.info('Windows unlocked, starting app now...') - setTimeout(startApp, 1000) + setTimeout(() => safeStartApp(0), 1000) }) return } } // Normal flow: start after 1 second - setTimeout(startApp, 1000) + setTimeout(() => safeStartApp(0), 1000) }) await attachProtocolListeners() @@ -300,12 +316,12 @@ function attachOnReadyProcess() { let waitingForConnection = false async function startApp(attempt = 0) { - let data = store.store || store.getFromDisk() + let data: LocalStorageData | null = store.store || store.getFromDisk() if (!checkData(data)) { if (attempt === 0) { data = store.getFromDisk() store.updateStore(data, 'startApp') - startApp(++attempt) + safeStartApp(++attempt) return; } else { await resetApp() @@ -337,7 +353,7 @@ function attachOnReadyProcess() { } retryAppStart = setTimeout(() => { - startApp(++attempt) + safeStartApp(++attempt) }, 1000) } else { if (retryAppStart) { @@ -382,6 +398,36 @@ function attachOnReadyProcess() { } } + /** + * startApp is always launched in a fire and forget fashion: without this guard an + * unexpected rejection - a corrupted user data file being the known case - leaves + * the splash screen up forever, since nothing would ever reach showLogin() nor + * SplashScreenController.quit(). Degrading to the login window is always better + * than a frozen "starting application" screen. + */ + function safeStartApp(attempt = 0) { + startApp(attempt).catch((error) => { + Log.error('START - unrecoverable error while starting the app', error) + try { + showLogin() + } catch (e) { + Log.error('START - unable to show the login window', e) + } + try { + SplashScreenController.instance.window.quit(true) + } catch (e) { + Log.error('START - unable to close the splash screen', e) + } + try { + TrayController.instance.updateTray({ + enableShowButton: true + }) + } catch (e) { + Log.error('START - unable to update the tray', e) + } + }) + } + // Polling interval for checking connection when waiting let connectionCheckInterval: NodeJS.Timeout | null = null @@ -404,7 +450,7 @@ function attachOnReadyProcess() { } catch (e) { // Splash screen might be closed } - startApp(0) + safeStartApp(0) } }, 5000) // Check every 5 seconds } @@ -421,7 +467,7 @@ function attachOnReadyProcess() { Log.info('START - user requested connection retry') waitingForConnection = false stopConnectionPolling() - startApp(0) + safeStartApp(0) }) app.on('window-all-closed', () => {