fix: Lade-Race in BoolPref/StringSetPref (Review-Befund)

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 <noreply@anthropic.com>
This commit is contained in:
Bernd Steckmeister
2026-07-03 23:20:49 +02:00
parent 03b84e0e0e
commit 7b90f20e36
3 changed files with 50 additions and 4 deletions
+5
View File
@@ -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
+13 -4
View File
@@ -7,6 +7,9 @@ import 'package:shared_preferences/shared_preferences.dart';
class BoolPref extends StateNotifier<bool> {
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<bool> {
Future<void> _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<void> 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<bool> {
class StringSetPref extends StateNotifier<Set<String>> {
final String _key;
late final Future<void> _loaded;
StringSetPref(this._key) : super({}) {
_load();
_loaded = _load();
}
Future<void> _load() async {
@@ -42,14 +47,18 @@ class StringSetPref extends StateNotifier<Set<String>> {
}
Future<void> 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<void> 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());
+32
View File
@@ -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();