From 7b90f20e36a66a0971d9a3f8065e4ae42c282327 Mon Sep 17 00:00:00 2001 From: Bernd Steckmeister Date: Fri, 3 Jul 2026 23:20:49 +0200 Subject: [PATCH] fix: Lade-Race in BoolPref/StringSetPref (Review-Befund) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StringSetPref.add/remove machten Read-Modify-Write auf möglicherweise noch ungeladenem State: ein add() direkt nach Erstellung persistierte nur den neuen Eintrag und LÖSCHTE alle bereits gespeicherten (per neuem Test am alten Code bewiesen). Jetzt warten add/remove auf _loaded. BoolPref bekommt einen _userSet-Guard, damit ein spät landender persistierter Wert nie eine schon getätigte Nutzeraktion zurücküberschreibt (Reihenfolge ist plattform- abhängig). In der Praxis kaum treffbar (Provider laden beim App-Start), aber die Fassade wird beim M2-Storage-Umbau Pflicht-Schnittstelle für neue, ggf. früh laufende Aufrufer. 2 neue Regressionstests, alle 24 grün. Co-Authored-By: Claude Fable 5 --- CHANGES.md | 5 +++++ lib/core/settings_prefs.dart | 17 +++++++++++++---- test/settings_prefs_test.dart | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 92336e2..fdffebe 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1212,3 +1212,8 @@ 2026-07-03 22:52 [Write] /home/steggi/pyramid/test/settings_prefs_test.dart 2026-07-03 22:53 [Edit] /home/steggi/pyramid/PROGRESS.md 2026-07-03 22:53 [Edit] /home/steggi/pyramid/docs/PC_TESTPLAN.md +2026-07-03 22:54 [Edit] /home/steggi/.claude-pyramid/projects/-home-steggi-pyramid/memory/pyramid-arbeitsstand.md +2026-07-03 23:18 [Edit] /home/steggi/pyramid/test/settings_prefs_test.dart +2026-07-03 23:18 [Edit] /home/steggi/pyramid/test/settings_prefs_test.dart +2026-07-03 23:19 [Edit] /home/steggi/pyramid/lib/core/settings_prefs.dart +2026-07-03 23:20 [Edit] /home/steggi/pyramid/lib/core/settings_prefs.dart diff --git a/lib/core/settings_prefs.dart b/lib/core/settings_prefs.dart index 7fd2ae4..505d26f 100644 --- a/lib/core/settings_prefs.dart +++ b/lib/core/settings_prefs.dart @@ -7,6 +7,9 @@ import 'package:shared_preferences/shared_preferences.dart'; class BoolPref extends StateNotifier { final String _key; + // Guards against the load race: if set()/toggle() ran before _load's read + // lands, the stale persisted value must not overwrite the user's choice. + bool _userSet = false; BoolPref(this._key, bool defaultValue) : super(defaultValue) { _load(defaultValue); @@ -14,10 +17,11 @@ class BoolPref extends StateNotifier { Future _load(bool def) async { final p = await SharedPreferences.getInstance(); - if (mounted) state = p.getBool(_key) ?? def; + if (mounted && !_userSet) state = p.getBool(_key) ?? def; } Future set(bool v) async { + _userSet = true; state = v; final p = await SharedPreferences.getInstance(); await p.setBool(_key, v); @@ -30,9 +34,10 @@ class BoolPref extends StateNotifier { class StringSetPref extends StateNotifier> { final String _key; + late final Future _loaded; StringSetPref(this._key) : super({}) { - _load(); + _loaded = _load(); } Future _load() async { @@ -42,14 +47,18 @@ class StringSetPref extends StateNotifier> { } Future add(String v) async { - if (state.contains(v)) return; + // Read-modify-write: ohne abgeschlossenes Laden würde hier der halbleere + // Start-State zurückgeschrieben und persistierte Einträge gingen verloren. + await _loaded; + if (!mounted || state.contains(v)) return; state = {...state, v}; final p = await SharedPreferences.getInstance(); await p.setStringList(_key, state.toList()); } Future remove(String v) async { - if (!state.contains(v)) return; + await _loaded; + if (!mounted || !state.contains(v)) return; state = {...state}..remove(v); final p = await SharedPreferences.getInstance(); await p.setStringList(_key, state.toList()); diff --git a/test/settings_prefs_test.dart b/test/settings_prefs_test.dart index a28684d..1981b92 100644 --- a/test/settings_prefs_test.dart +++ b/test/settings_prefs_test.dart @@ -23,6 +23,21 @@ void main() { expect(pref.state, false, reason: 'persistierter Wert muss gewinnen'); }); + test( + 'set() sofort nach Erstellung gewinnt gegen den später ladenden ' + 'persistierten Wert (Lade-Race)', () async { + SharedPreferences.setMockInitialValues({'notif_sound': false}); + final pref = BoolPref('notif_sound', false); + final done = pref.set(true); // bevor _load fertig ist + await pumpEventQueue(); + await done; + expect(pref.state, true, + reason: 'die Nutzeraktion darf nicht vom Lade-Ergebnis ' + 'zurücküberschrieben werden'); + final p = await SharedPreferences.getInstance(); + expect(p.getBool('notif_sound'), true); + }); + test('set/toggle ändern Zustand und persistieren', () async { final pref = BoolPref('notif_desktop', true); await pumpEventQueue(); @@ -49,6 +64,23 @@ void main() { expect(p.getStringList('muted_rooms')!.toSet(), {'!b:hs', '!c:hs'}); }); + test( + 'add() sofort nach Erstellung verliert keine bereits persistierten ' + 'Einträge (Lade-Race)', () async { + SharedPreferences.setMockInitialValues({ + 'muted_rooms': ['!a:hs'], + }); + final pref = StringSetPref('muted_rooms'); + final done = pref.add('!b:hs'); // bevor _load fertig ist + await pumpEventQueue(); + await done; + expect(pref.state, {'!a:hs', '!b:hs'}); + final p = await SharedPreferences.getInstance(); + expect(p.getStringList('muted_rooms')!.toSet(), {'!a:hs', '!b:hs'}, + reason: 'Read-Modify-Write auf ungeladenem State darf keine ' + 'persistierten Einträge verlieren'); + }); + test('doppeltes add und remove von Fehlendem sind No-Ops', () async { final pref = StringSetPref('muted_rooms'); await pumpEventQueue();