Skip to content

Possible memory leaks in watch method #5

@jinyus

Description

@jinyus

Forgive me if I misunderstood the code, but these lines registers a new effect everytime signalName.watch(context) is called ie: when the widget rebuilds.

https://github.com/rodydavis/preact_signals.dart/blob/08e958f67108f680920d78369c7c32dc146e25c1/packages/flutter_preact_signals/lib/src/extensions/watch.dart#L12C1-L23C2

Notice the increase in the number of times the effect runs on each rebuild:

Screenshot_20231122_111152

If I'm correct, you'd need to keep track of which widget is subscribed to each signal and check this before registering a new effect...This quick hack solved the issue:

final Set<String> _subscribers = {};
/// Watch a signal value and rebuild the context of the [Element]
/// if mounted and mark it as dirty
T watchSignal<T>(BuildContext context, ReadonlySignal<T> signal) {
  if (context is Element) {
    final elementRef = WeakReference(context);
    final key = '${signal.hashCode} ${elementRef.target.hashCode}';

    // checks if the widget is already subscribed to the signal
    if (!_subscribers.contains(key)) {
      print('watching signal value:${signal.peek()} |  id:${signal.hashCode}');

      _subscribers.add(key);
      signal.subscribe((_) {
        print('effect signal value:${signal.peek()} executed');
        if (elementRef.target?.mounted ?? false) {
          elementRef.target?.markNeedsBuild();
        }
      });
    }
  }
  return signal.peek();
}

Now the effect only runs once,triggering the rebuld:

Screenshot_20231122_111923

It also seems that the EffectCleanup returned from effect is never called...introducing another memory leak.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions