rrousselGit / state_notifier

ValueNotifier, but outside Flutter and with some extra perks
MIT License
311 stars 28 forks source link

Removing a listener inside itself causes an exception #35

Closed kaboc closed 1 year ago

kaboc commented 4 years ago

It causes an exception to remove a listener inside itself using the function returned from addListener() of StateNotifier.

To Reproduce

Run the code below and see the output.

class Counter extends StateNotifier<int> {
  Counter() : super(0);

  void increment() => state++;
}
Function _removeCounterListener;
Timer _timer;

void main() {
  final counter = Counter();
  _removeCounterListener = counter.addListener(_counterListener);

  _timer = Timer.periodic(const Duration(seconds: 1), (_) {
    counter.increment();
  });
}

void _counterListener(int state) {
  print('Counter: $state');

  if (state == 3) {
    _removeCounterListener();
    _timer.cancel();
  }
}

Output:

Counter: 0
Counter: 1
Counter: 2
Counter: 3
Unhandled exception:
Concurrent modification during iteration: Instance of '_LinkedListIterator<_ListenerEntry<int>>'.
#0      _LinkedListIterator.moveNext (dart:collection/linked_list.dart:191:7)
#1      StateNotifier.state= (package:state_notifier/state_notifier.dart:160:33)
#2      Counter.increment (file:///D:/state_notifier_issue/main.dart:29:23)
#3      main.<anonymous closure> (file:///D:/state_notifier_issue/main.dart:13:13)
#4      _Timer._runTimers (dart:isolate-patch/timer_impl.dart:397:19)
#5      _Timer._handleMessage (dart:isolate-patch/timer_impl.dart:428:5)
#6      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:168:12)

Expected behavior

No exception occurs.

kaboc commented 4 years ago

I'm not sure if it is intentional, but what is possible with Stream (as follows) is impossible with StateNotifier's addListener(), which seems inconvenient and confusing for me.

StreamSubscription _counterSubscription;
Timer _timer;

void main() {
  final counter = Counter();
  _counterSubscription = counter.stream.listen(_counterListener);

  _timer = Timer.periodic(const Duration(seconds: 1), (_) {
    counter.increment();
  });
}

void _counterListener(int state) {
  print('Counter: $state');

  if (state == 3) {
    _counterSubscription.cancel();
    _timer.cancel();
  }
}

Output:

Counter: 1
Counter: 2
Counter: 3
rrousselGit commented 4 years ago

It's intentional But we can do something about it.

I made a PR on Flutter about optimizing ValueNotifier, which supports this use-case. We could do the same thing here too

Reprevise commented 1 year ago

Still an issue to this day, can we do something about it?

rrousselGit commented 1 year ago

No, I don't plan on changing it as this point.