searchfield icon indicating copy to clipboard operation
searchfield copied to clipboard

removeOverlay condition too strict

Open franzlst opened this issue 1 year ago • 13 comments

Describe the bug I have a strange bug that does not always occur, but which seems to be some kind of race condition. However, it is quite severe and leads to a freeze of the whole Flutter app. So far, I was only able to reproduce it in the web build of my app, not in any debug builds. The error logs are not so helpful:

08:44:22.610 Bad state: RenderBox was not laid out: minified:am0#1cb7a [main.dart.js:48094:78](https://***.com/main.dart.js)
08:44:22.611 <empty string> [main.dart.js:48094:78](https://***.com/main.dart.js)
08:44:22.615 Another exception was thrown: Instance of 'minified:lb<void>' 334 [main.dart.js:48094:78](https://***.com/main.dart.js)

The error occurs when repeatedly clicking inside and outside of a SearchField.

However, in the debug builds I was able to sporadically create an error, which might be related to the crash:

======== Exception caught by foundation library ====================================================
The following assertion was thrown while dispatching notifications for FocusNode:
The specified entry is already present in the target Overlay.

The OverlayEntry was: OverlayEntry#fce75(opaque: false; maintainState: false)
The Overlay the OverlayEntry was trying to insert to was: OverlayState#1542a(entries: [OverlayEntry#5ea75(opaque: true; maintainState: false), OverlayEntry#8bd21(opaque: false; maintainState: true), OverlayEntry#fce75(opaque: false; maintainState: false)])
  entries: [OverlayEntry#5ea75(opaque: true; maintainState: false), OverlayEntry#8bd21(opaque: false; maintainState: true), OverlayEntry#fce75(opaque: false; maintainState: false)]

Consider calling remove on the OverlayEntry before inserting it to a different Overlay, or switching to the OverlayPortal API to avoid manual OverlayEntry management.

When the exception was thrown, this was the stack: 
dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 296:3  throw_
packages/flutter/src/widgets/overlay.dart 632:7                              [_debugCanInsertEntry]
packages/flutter/src/widgets/overlay.dart 670:12                             insert
packages/searchfield/src/searchfield.dart 465:20                             <fn>
packages/flutter/src/foundation/change_notifier.dart 437:24                  notifyListeners
packages/flutter/src/widgets/focus_manager.dart 1090:5                       [_notify]
packages/flutter/src/widgets/focus_manager.dart 1871:11                      applyFocusChangesIfNeeded
dart-sdk/lib/async/schedule_microtask.dart 40:11                             _microtaskLoop
dart-sdk/lib/async/schedule_microtask.dart 49:5                              _startMicrotaskLoop
dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 181:7           <fn>
The FocusNode sending notification was: FocusNode#f6638([PRIMARY FOCUS])
  context: Focus
  PRIMARY FOCUS
====================================================================================================

By adding some debug print statements, I think I was able to pin down the cause of the exception. The current implementation of removeOverlay() is the following:

  void removeOverlay() {
    if (_overlayEntry != null && _overlayEntry!.mounted) {
      isSuggestionsShown = false;
      if (_overlayEntry != null) {
        _overlayEntry?.remove();
      }
    }
  }

So the overlay is only removed, when it is mounted. This causes the overlay not always to be removed, as the widget might not be removed immediately:

After the owner of the OverlayEntry calls remove and then dispose, the widget may not be immediately removed from the widget tree. As a result listeners of the OverlayEntry can get notified for one last time after the dispose call, when the widget is eventually unmounted.

From: https://api.flutter.dev/flutter/widgets/OverlayEntry-class.html

After removing the condition && _overlayEntry!.mounted, I was no longer able to reproduce the issue locally.

  • [x] By clicking this checkbox, I confirm I am using the latest version of the package found on pub.dev/searchfield

franzlst avatar Oct 11 '24 07:10 franzlst