-
Notifications
You must be signed in to change notification settings - Fork 75
Description
Environment
- Flutter: 3.38.3
- Dart: 3.10.1
- signals: 6.2.0
- Platform: macOS, iOS, Android, Web (all affected)
Description
SignalsMixin._setup() iterates lazily over _signals.values.where((e) => e.local != null) while accessing signal values. If a signal value access triggers bindSignal() or _watch() during that iteration, the _signals HashMap is modified, causing ConcurrentModificationError.
Root Cause
File: packages/signals_flutter/lib/src/mixins/signals.dart, line ~65
Current code (problematic):
final cb = effect(() {
for (final s in _signals.values.where((e) => e.local != null)) { // ← Lazy iterable
s.target.value; // ← Can trigger _watch() → modifies _signals
}
_rebuild();
});The issue: HashMap.values returns a lazy Iterable, not a snapshot. During iteration, if accessing s.target.value triggers bindSignal() on a new signal (e.g., through a computed that reads an external signal), _watch() mutates _signals while the iterator is active.
Minimal Reproduction
import 'package:flutter/material.dart';
import 'package:signals/signals_flutter.dart';
void main() => runApp(const MaterialApp(home: BugRepro()));
// External signal (not created via SignalsMixin)
final externalSignal = signal<int>(0);
class BugRepro extends StatefulWidget {
const BugRepro({super.key});
@override
State<BugRepro> createState() => _BugReproState();
}
class _BugReproState extends State<BugRepro> with SignalsMixin {
late final localSignal = createSignal<int>(0);
@override
void initState() {
super.initState();
createEffect(() {
final local = localSignal.value;
// First access to externalSignal during effect iteration
// triggers bindSignal → _watch → HashMap mutation
final external = bindSignal(externalSignal).value; // 💥
debugPrint('local: $local, external: $external');
});
}
@override
Widget build(BuildContext context) {
return ElevatedButton(
onPressed: () => localSignal.value++,
child: const Text('Increment'),
);
}
}Alternative trigger (more common in practice):
// Computed that depends on both local AND external signals
late final combined = createComputed(() {
return localSignal.value + externalSignal.value; // externalSignal access triggers _watch
});
createEffect(() {
debugPrint('Combined: ${combined.value}'); // 💥 First access
});Stack Trace
ConcurrentModificationError: Concurrent modification during iteration
at _CompactIterable.iterator
at SignalsMixin._setup.<effect>
at SignalsEffectScheduler._runEffects
...
Proposed Fix
Copy to list before iterating to create a snapshot:
final cb = effect(() {
// FIX: .toList() creates snapshot, prevents concurrent modification
final signalsToTrack = _signals.values.where((e) => e.local != null).toList();
for (final s in signalsToTrack) {
s.target.value;
}
_rebuild();
});Same fix needed in clearSignalsAndEffects():
void clearSignalsAndEffects() {
_cleanup?.call();
_cleanup = null;
// FIX: Copy before iteration
final local = _signals.values
.where((e) => e.local ?? false)
.map((e) => e.target)
.toList();
for (final s in local) {
s.dispose();
}
// FIX: Copy effects before iteration
final effectsCopy = List<EffectCleanup>.from(_effects);
for (final cb in effectsCopy) {
cb();
}
_effects.clear();
_signals.clear();
}Workaround
We created SafeSignalsMixin as a drop-in replacement with the fix applied. Happy to submit a PR with the fix.