Skip to content

Fix memory leak in NestedScrollViewState. [prod-leak-fix]#135248

Merged
polina-c merged 3 commits intoflutter:masterfrom
ksokolovskyi:nested-scroll-view-leak
Sep 22, 2023
Merged

Fix memory leak in NestedScrollViewState. [prod-leak-fix]#135248
polina-c merged 3 commits intoflutter:masterfrom
ksokolovskyi:nested-scroll-view-leak

Conversation

@ksokolovskyi
Copy link
Contributor

@ksokolovskyi ksokolovskyi commented Sep 21, 2023

This PR fixes a memory leak in the NestedScrollViewState, which did not dispose created SliverOverlapAbsorberHandle.

Description

Tests

  • Adds a new test to nested_scroll_view_test.dart to verify that SliverOverlapAbsorberHandle dispatches memory events;
  • Updates nested_scroll_view_test.dart to use testWidgetsWithLeakTracking.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Sep 21, 2023
@ksokolovskyi ksokolovskyi changed the title Fix memory leak in NestedScrollViewState Fix memory leak in NestedScrollViewState. Sep 21, 2023
@ksokolovskyi
Copy link
Contributor Author

ksokolovskyi commented Sep 21, 2023

cc @polina-c

@polina-c polina-c added the a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker label Sep 21, 2023
@polina-c polina-c self-requested a review September 21, 2023 23:34
@polina-c
Copy link
Contributor

polina-c commented Sep 22, 2023

Google testing failed because it did not catch up with conversion of maybeDispatchObjectCreation to static:

ERROR: third_party/dart/flutter/lib/src/widgets/nested_scroll_view.dart

1627: ChangeNotifier.maybeDispatchObjectCreation(this); Instance member 'maybeDispatchObjectCreation' can't be accessed using static access.
                                                        Too many positional arguments: 0 expected, but 1 found.
                                                        #static_access_to_instance_member #extra_positional_arguments


google3:///[third_party/dart/flutter/lib/src/widgets/nested_scroll_view.dart:1627](https://cs.corp.google.com/piper///depot/google3/third_party/dart/flutter/lib/src/widgets/nested_scroll_view.dart?l=1627&ws=tap-presubmit-server/107738340&snapshot=5):22: Error: Member not found: 'ChangeNotifier.maybeDispatchObjectCreation'.
      ChangeNotifier.maybeDispatchObjectCreation(this);
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^

@polina-c polina-c merged commit 9d42ad8 into flutter:master Sep 22, 2023
@ksokolovskyi
Copy link
Contributor Author

ksokolovskyi commented Sep 22, 2023

@polina-c thanks for the review!

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 22, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 22, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
1e58fa3 
…5248)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 22, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 22, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
baf864d 
…5248)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 22, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 22, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
5123844 
…5248)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 22, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 22, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
ab7538c 
…5248)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 22, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 22, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
1759e52 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
481c63b 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
059dd9c 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
a78e31f 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
2ad5b9e 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
5d6c80d 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 26, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
2690ecd 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
d13888b 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
e9da6e7 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
697f8be 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
0fc27aa 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 27, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
127b5b8 
…5248)
@HansMuller HansMuller mentioned this pull request Sep 28, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
639cb26 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
f245af2 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
23ece4c 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
974c929 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
8a297fd 
…5248)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 28, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
644737b 
…5248)
Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023
@ksokolovskyi @Mairramer
Fix memory leak in NestedScrollViewState. (flutter#135248)
67ac77d
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
@engine-flutter-autoroll
9d42ad8 Fix memory leak in NestedScrollViewState. (flutter/flutter#13…
b04ffa0 
…5248)
@polina-c polina-c changed the title Fix memory leak in NestedScrollViewState. Fix memory leak in NestedScrollViewState. [prod-leak-fix] Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@polina-c polina-c polina-c approved these changes

Assignees

No one assigned

Labels

a: leak tracking Issues and PRs related to memory leaks detected by leak_tracker f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory Leak: NestedScrollViewState does not dispose SliverOverlapAbsorberHandle.

2 participants

@ksokolovskyi @polina-c