From a711aadeed283da1b870e0b29d1ab2b7e5283775 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:22:27 +0000 Subject: [PATCH 1/6] fix: write persistence files atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FileDataStoreConfig persisted both data stores and the resolver index with file.writeAsString, which truncates the target before writing the new contents. A crash, OOM kill, or power loss mid-write left the file torn — on the next launch jsonDecode throws (only PathNotFoundException is caught), so that partition's data is lost or hydration breaks. This is the default persistor on mobile/desktop. Write to a sibling temp file, flush it to disk, then rename over the target. A same-filesystem rename is atomic, so the target always holds either the complete previous contents or the complete new contents. Modern Dart's File.rename replaces an existing regular file on both POSIX and Windows, so no platform special-casing is needed. The .tmp suffix is not matched by the worker's data store file listing, so a temp file orphaned by an interrupted write is ignored and overwritten on the next write rather than loaded as a store. --- .../file_data_store_config.dart | 33 +++++++++++- test/native/file_persistor_test.dart | 52 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/lib/persistor/file_persistor/file_data_store_config.dart b/lib/persistor/file_persistor/file_data_store_config.dart index 7a5a2ee..1ad6235 100644 --- a/lib/persistor/file_persistor/file_data_store_config.dart +++ b/lib/persistor/file_persistor/file_data_store_config.dart @@ -4,6 +4,33 @@ import 'package:loon/loon.dart'; import 'package:loon/persistor/data_store.dart'; import 'package:loon/persistor/data_store_resolver.dart'; +/// Writes [contents] to [file] atomically: the data is written to a sibling +/// temporary file, flushed to disk, then renamed over the target. A rename on +/// the same filesystem is atomic, so an interrupted write (crash, OOM kill, +/// power loss) can never leave the target torn — it always holds either the +/// complete previous contents or the complete new contents. A plain +/// `writeAsString` truncates the target up front, leaving a window where a +/// crash corrupts it. +/// +/// The `.tmp` suffix is deliberately not matched by the data store file listing +/// (`fileRegex` in the worker), so a temp file orphaned by an interrupted write +/// is ignored and overwritten on the next write rather than loaded as a store. +Future _writeFileAtomic(File file, String contents) async { + final tmpFile = File('${file.path}.tmp'); + + final raf = await tmpFile.open(mode: FileMode.writeOnly); + try { + await raf.writeString(contents); + // Flush the data to disk before the rename so the rename can't expose an + // empty/partial file after a power loss. + await raf.flush(); + } finally { + await raf.close(); + } + + await tmpFile.rename(file.path); +} + class FileDataStoreConfig extends DataStoreConfig { FileDataStoreConfig( super.name, { @@ -34,7 +61,8 @@ class FileDataStoreConfig extends DataStoreConfig { persist: (store) async { final value = jsonEncode(store.extract()); - await file.writeAsString( + await _writeFileAtomic( + file, encrypted ? encrypter.encrypt(value) : value, ); }, @@ -61,7 +89,8 @@ class FileDataStoreResolverConfig extends DataStoreResolverConfig { return null; } }, - persist: (store) => file.writeAsString(jsonEncode(store.inspect())), + persist: (store) => + _writeFileAtomic(file, jsonEncode(store.inspect())), delete: () async { try { await file.delete(); diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index fdfbb17..fd82bb3 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -1,7 +1,13 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:encrypt/encrypt.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:loon/loon.dart'; +import 'package:loon/persistor/data_store_encrypter.dart'; import 'package:loon/persistor/file_persistor/file_persistor.dart'; +import 'package:loon/persistor/file_persistor/file_persistor_worker.dart' + show fileRegex; // ignore: depend_on_referenced_packages import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; @@ -53,4 +59,50 @@ void main() { factory: FilePersistor.new, ); }); + + group('FilePersistor durability', () { + final loonDir = Directory('${testDirectory.path}/loon'); + + // FlutterSecureStorage does not work in tests, so supply a fixed encrypter. + final encrypter = DataStoreEncrypter( + Encrypter(AES(Key.fromSecureRandom(32), mode: AESMode.cbc)), + ); + + tearDown(() async { + await Loon.clearAll(); + }); + + test('Persists atomically, leaving no .tmp files behind', () async { + final synced = Completer(); + final persistor = FilePersistor( + persistenceThrottle: const Duration(milliseconds: 1), + encrypter: encrypter, + onSync: () { + if (!synced.isCompleted) synced.complete(); + }, + ); + Loon.configure(persistor: persistor); + + Loon.collection('users').doc('1').create({'name': 'User 1'}); + await synced.future; + + expect(await File('${loonDir.path}/__store__.json').exists(), true); + + final tmpFiles = loonDir + .listSync() + .whereType() + .where((file) => file.path.endsWith('.tmp')) + .toList(); + expect(tmpFiles, isEmpty); + }); + + test('Data store file listing ignores orphaned .tmp files', () { + // A temp file orphaned by an interrupted atomic write must not be picked + // up as a data store on the next startup. + expect(fileRegex.hasMatch('users.json'), true); + expect(fileRegex.hasMatch('users.encrypted.json'), true); + expect(fileRegex.hasMatch('users.json.tmp'), false); + expect(fileRegex.hasMatch('__store__.json.tmp'), false); + }); + }); } From 25fb6229f94bf7ce12e98b6aa7368735bb89a9c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 15:40:52 +0000 Subject: [PATCH 2/6] ci: retrigger (pre-existing test/core timing flake, unrelated to atomic writes) From a55b2a71ce172ccefe2926aa26f84c6e3715f37a Mon Sep 17 00:00:00 2001 From: Dan Reynolds Date: Sun, 31 May 2026 08:29:27 -0400 Subject: [PATCH 3/6] Annotate file persistor test path provider --- test/native/file_persistor_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index fd82bb3..72d599d 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -20,7 +20,7 @@ late Directory testDirectory; class MockPathProvider extends Fake with MockPlatformInterfaceMixin implements PathProviderPlatform { - getApplicationDocumentsDirectory() { + Directory getApplicationDocumentsDirectory() { return testDirectory; } From 9fe3887f6b30da7b28bb76fb90cdce03a5b9e8a0 Mon Sep 17 00:00:00 2001 From: Dan Reynolds Date: Sun, 31 May 2026 08:36:47 -0400 Subject: [PATCH 4/6] Clean stale atomic write temp files --- .../file_data_store_config.dart | 29 +++++++++++++------ .../file_persistor/file_persistor_worker.dart | 21 ++++++++++++++ test/native/file_persistor_test.dart | 15 ++++++++++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/lib/persistor/file_persistor/file_data_store_config.dart b/lib/persistor/file_persistor/file_data_store_config.dart index 1ad6235..a0c3827 100644 --- a/lib/persistor/file_persistor/file_data_store_config.dart +++ b/lib/persistor/file_persistor/file_data_store_config.dart @@ -18,17 +18,28 @@ import 'package:loon/persistor/data_store_resolver.dart'; Future _writeFileAtomic(File file, String contents) async { final tmpFile = File('${file.path}.tmp'); - final raf = await tmpFile.open(mode: FileMode.writeOnly); try { - await raf.writeString(contents); - // Flush the data to disk before the rename so the rename can't expose an - // empty/partial file after a power loss. - await raf.flush(); - } finally { - await raf.close(); - } + final raf = await tmpFile.open(mode: FileMode.writeOnly); + try { + await raf.writeString(contents); + // Flush the data to disk before the rename so the rename can't expose an + // empty/partial file after a power loss. + await raf.flush(); + } finally { + await raf.close(); + } - await tmpFile.rename(file.path); + await tmpFile.rename(file.path); + } catch (_) { + // Best-effort cleanup for normal write failures. This will not run after a + // process crash; startup cleanup in the file worker handles those leftovers. + try { + await tmpFile.delete(); + } on FileSystemException { + // Preserve the original write error if cleanup also fails. + } + rethrow; + } } class FileDataStoreConfig extends DataStoreConfig { diff --git a/lib/persistor/file_persistor/file_persistor_worker.dart b/lib/persistor/file_persistor/file_persistor_worker.dart index e597936..61e4e6c 100644 --- a/lib/persistor/file_persistor/file_persistor_worker.dart +++ b/lib/persistor/file_persistor/file_persistor_worker.dart @@ -9,6 +9,26 @@ import 'package:path/path.dart' as path; final fileRegex = RegExp(r'^(?!__resolver__)(\w+?)(?:.encrypted)?\.json$'); +Future _deleteTempFiles(Directory directory) async { + try { + final tmpFiles = directory + .listSync() + .whereType() + .where((file) => path.basename(file.path).endsWith('.tmp')); + + await Future.wait(tmpFiles.map((file) async { + try { + await file.delete(); + } on FileSystemException { + // Stale temp files are ignored by hydration, so cleanup should not + // prevent startup if a file is already gone or cannot be removed. + } + })); + } on PathNotFoundException { + return; + } +} + class FilePersistorWorkerConfig extends PersistorWorkerConfig { final Directory directory; @@ -66,6 +86,7 @@ class FilePersistorWorker extends PersistorWorker { @override init() async { + await _deleteTempFiles(config.directory); await _manager.init(); } diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index 72d599d..82055a7 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -104,5 +104,20 @@ void main() { expect(fileRegex.hasMatch('users.json.tmp'), false); expect(fileRegex.hasMatch('__store__.json.tmp'), false); }); + + test('Cleans orphaned .tmp files on startup', () async { + final orphanedStoreTmp = File('${loonDir.path}/users.json.tmp'); + final orphanedResolverTmp = File('${loonDir.path}/__resolver__.json.tmp'); + await orphanedStoreTmp.writeAsString('partial store write'); + await orphanedResolverTmp.writeAsString('partial resolver write'); + + Loon.configure( + persistor: FilePersistor(encrypter: encrypter), + ); + await Loon.hydrate(); + + expect(await orphanedStoreTmp.exists(), false); + expect(await orphanedResolverTmp.exists(), false); + }); }); } From bb8e9c70ec2149ac0d9df818ad558850b54b92be Mon Sep 17 00:00:00 2001 From: Dan Reynolds Date: Sun, 31 May 2026 08:42:38 -0400 Subject: [PATCH 5/6] Tie temp cleanup to atomic writes --- .../file_data_store_config.dart | 11 +++++++-- .../file_persistor/file_persistor_worker.dart | 21 ----------------- test/native/file_persistor_test.dart | 23 +++++++++++++++---- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lib/persistor/file_persistor/file_data_store_config.dart b/lib/persistor/file_persistor/file_data_store_config.dart index a0c3827..9c85cd5 100644 --- a/lib/persistor/file_persistor/file_data_store_config.dart +++ b/lib/persistor/file_persistor/file_data_store_config.dart @@ -14,10 +14,17 @@ import 'package:loon/persistor/data_store_resolver.dart'; /// /// The `.tmp` suffix is deliberately not matched by the data store file listing /// (`fileRegex` in the worker), so a temp file orphaned by an interrupted write -/// is ignored and overwritten on the next write rather than loaded as a store. +/// is ignored and deleted before the next write to the same target rather than +/// loaded as a store. Future _writeFileAtomic(File file, String contents) async { final tmpFile = File('${file.path}.tmp'); + try { + await tmpFile.delete(); + } on PathNotFoundException { + // No stale temp file from a previous interrupted write. + } + try { final raf = await tmpFile.open(mode: FileMode.writeOnly); try { @@ -32,7 +39,7 @@ Future _writeFileAtomic(File file, String contents) async { await tmpFile.rename(file.path); } catch (_) { // Best-effort cleanup for normal write failures. This will not run after a - // process crash; startup cleanup in the file worker handles those leftovers. + // process crash; the next write to this target removes the stale temp file. try { await tmpFile.delete(); } on FileSystemException { diff --git a/lib/persistor/file_persistor/file_persistor_worker.dart b/lib/persistor/file_persistor/file_persistor_worker.dart index 61e4e6c..e597936 100644 --- a/lib/persistor/file_persistor/file_persistor_worker.dart +++ b/lib/persistor/file_persistor/file_persistor_worker.dart @@ -9,26 +9,6 @@ import 'package:path/path.dart' as path; final fileRegex = RegExp(r'^(?!__resolver__)(\w+?)(?:.encrypted)?\.json$'); -Future _deleteTempFiles(Directory directory) async { - try { - final tmpFiles = directory - .listSync() - .whereType() - .where((file) => path.basename(file.path).endsWith('.tmp')); - - await Future.wait(tmpFiles.map((file) async { - try { - await file.delete(); - } on FileSystemException { - // Stale temp files are ignored by hydration, so cleanup should not - // prevent startup if a file is already gone or cannot be removed. - } - })); - } on PathNotFoundException { - return; - } -} - class FilePersistorWorkerConfig extends PersistorWorkerConfig { final Directory directory; @@ -86,7 +66,6 @@ class FilePersistorWorker extends PersistorWorker { @override init() async { - await _deleteTempFiles(config.directory); await _manager.init(); } diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index 82055a7..00073d5 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -105,16 +105,31 @@ void main() { expect(fileRegex.hasMatch('__store__.json.tmp'), false); }); - test('Cleans orphaned .tmp files on startup', () async { - final orphanedStoreTmp = File('${loonDir.path}/users.json.tmp'); + test('Cleans orphaned .tmp files before rewriting their targets', () async { + final orphanedStoreTmp = File('${loonDir.path}/custom_store.json.tmp'); final orphanedResolverTmp = File('${loonDir.path}/__resolver__.json.tmp'); await orphanedStoreTmp.writeAsString('partial store write'); await orphanedResolverTmp.writeAsString('partial resolver write'); + final synced = Completer(); Loon.configure( - persistor: FilePersistor(encrypter: encrypter), + persistor: FilePersistor( + persistenceThrottle: const Duration(milliseconds: 1), + encrypter: encrypter, + onSync: () { + if (!synced.isCompleted) synced.complete(); + }, + ), + ); + + final users = Loon.collection( + 'users', + persistorSettings: PersistorSettings( + key: Persistor.key('custom_store'), + ), ); - await Loon.hydrate(); + users.doc('1').create({'name': 'User 1'}); + await synced.future; expect(await orphanedStoreTmp.exists(), false); expect(await orphanedResolverTmp.exists(), false); From b6b37803fb0fd24cee507702d3a6eb641e732d7b Mon Sep 17 00:00:00 2001 From: Dan Reynolds Date: Sun, 31 May 2026 08:51:19 -0400 Subject: [PATCH 6/6] Avoid extra temp-file delete before writes --- .../file_persistor/file_data_store_config.dart | 10 ++-------- test/native/file_persistor_test.dart | 3 ++- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/persistor/file_persistor/file_data_store_config.dart b/lib/persistor/file_persistor/file_data_store_config.dart index 9c85cd5..4135d75 100644 --- a/lib/persistor/file_persistor/file_data_store_config.dart +++ b/lib/persistor/file_persistor/file_data_store_config.dart @@ -14,17 +14,11 @@ import 'package:loon/persistor/data_store_resolver.dart'; /// /// The `.tmp` suffix is deliberately not matched by the data store file listing /// (`fileRegex` in the worker), so a temp file orphaned by an interrupted write -/// is ignored and deleted before the next write to the same target rather than +/// is ignored and overwritten by the next write to the same target rather than /// loaded as a store. Future _writeFileAtomic(File file, String contents) async { final tmpFile = File('${file.path}.tmp'); - try { - await tmpFile.delete(); - } on PathNotFoundException { - // No stale temp file from a previous interrupted write. - } - try { final raf = await tmpFile.open(mode: FileMode.writeOnly); try { @@ -39,7 +33,7 @@ Future _writeFileAtomic(File file, String contents) async { await tmpFile.rename(file.path); } catch (_) { // Best-effort cleanup for normal write failures. This will not run after a - // process crash; the next write to this target removes the stale temp file. + // process crash; the next write to this target overwrites the stale temp. try { await tmpFile.delete(); } on FileSystemException { diff --git a/test/native/file_persistor_test.dart b/test/native/file_persistor_test.dart index 00073d5..5cf223e 100644 --- a/test/native/file_persistor_test.dart +++ b/test/native/file_persistor_test.dart @@ -105,7 +105,8 @@ void main() { expect(fileRegex.hasMatch('__store__.json.tmp'), false); }); - test('Cleans orphaned .tmp files before rewriting their targets', () async { + test('Overwrites orphaned .tmp files when rewriting their targets', + () async { final orphanedStoreTmp = File('${loonDir.path}/custom_store.json.tmp'); final orphanedResolverTmp = File('${loonDir.path}/__resolver__.json.tmp'); await orphanedStoreTmp.writeAsString('partial store write');