leak-tracker: Detect memory leaks by identifying not disposed or not GCed objects.

[polina-c]

Motivation: go/flutter-devtools-survey-2022

[polina-c]

• Investigate
  • understand WeakMap
  • understand Finalizer API (Daco Harkes)
  • understand app to DevTools connection
  • how to connect DevTools to a test (no way)
  • create design (public, private)
  • review design
  • present to the team - go/memory-leak-detector-high-level-design
• Prototype
  • prototype the detector
  • pilot detector with Gallery
  • rump up with Flutter@Google3
  • ramp up on memory app
  • pilot detector with memory app
  • learn to pass GC events to the app
  • learn to find retention path
  • switch to GC event based approach
  • switch to combination of GC event and time based approach
  • figure out what to do with WeakReference in the path
  • show only root objects
  • request GC API changes
• Make available for early adopters
  • prepare patch for Flutter/Dart (layer.dart, framework.dart - state, element)
    • branch in GitHub for Flutter
    • CL in Google3
  • tab 'Leaks' in DevTools
    • shared library
    • summary in console
    • analyze and save
  • guidance
  • add link to documentation
  • review UX with UX specialists: go/notes-ux-review-for-memory-leaks-pane
• Pilot with early adopters
  • Gallery
  • work out leaks in debug mode
  • pilot and benchmark money app - go/leaktracker-for-money-app
  • DevTools
• Critical Improvements/Researches:
  • Make the GC signal reliable by adopting gcType: Add information about type of GC to the GC event details. dart-lang/sdk#49319
  • add section about performance impact to documentation
• Release leak tracker beta
  • Design doc
  • MVP
    • Instrument Image and Picture in dart.ui - Instrument Image and Picture for leak tracking. engine#35274
      • Work around Picture.toImage fails for --web-renderer html flutter#110599
      • Pre-work: Test cover 'toImage'. engine#35791
    • Create MemoryAllocations - Create class MemoryAllocations. flutter#110230
    • Instrument flutter classes: Instrument State, Layer, RenderObject and Element. flutter#111328
    • Figure out regression
    • Create package:leak_tracker (in https://github.com/orgs/dart-lang/repositories, with go/dart-lang-repo-creation-process)
      • create launch: https://github.com/dart-lang/leak_tracker
      • create code
        • leak tracking logic
        • integration tests
    • request gcCount: Implement reachability barrier dart-lang/sdk#50080
    • integrated with DevTools
    • port to Google3 - Port leak_tracker to Google3. #4894
    • Update documentation and link from documentation site
• Flutter regression - Add test coverage for non-disposed and non-GCed memory leaks. flutter#108322
  • Detect and fix some leaks: ValueNotifier is not disposed by tooltip. flutter#117063
  • discuss with flutter engineers: go/notes-leak-regression-for-flutter
  • discuss regression: go/notes-michael-polina-making-leak-tracker-dev-dependency
  • Discuss coverage - go/flutter-leak-tracking-test-coverage
  • Regression for animation, foundation and gestures, with not-GCed off: Define testWidgetsWithLeakTracking. flutter#125063
  • Create and link guidance to troubleshoot non-disposed memory leaks in the test failure
  • Create bugs for 13 non-disposed cases (hoping, that flutter people will find capacity for them)
    • Fix not-disposed memory leaks in animation_sheet_test.dart. flutter#126096
    • Fix not-disposed memory leaks in live_binding_test.dart. flutter#126097
    • Fix not-disposed memory leaks in gesture_config_regression_test.dart flutter#126098
    • Fix non-disposed ValueNotifier<_OverlayEntryWidgetState?> memory leak flutter#126100
    • Fix not-disposed memory leaks in flutter_logo_test.dart  flutter#126102
  • address feedback: Improve leak debugging UX. dart-lang/leak_tracker#59
  • fix flakiness: Leak Tracking is flaky flutter#126259
  • see retaining paths for non-gced objects in tests
    • figure out Result of getRetainingPath should not include WeakReference dart-lang/sdk#49155
    • figure out Service.getInfo() does not work for test environment. dart-lang/sdk#52354
    • figure out flutter test should support flag '--debug' flutter#127331
    • collect retaining path: Collect retaining path. dart-lang/leak_tracker#63
    • figure out Result of getRetainingPath should not be about the getRetainingPath invocation. dart-lang/sdk#52517
    • figure out Watcher fails with class 'FileSystemEvent' can't be extended, for dart, not for flutter. dart-lang/sdk#52570
    • nicely format retaining path: Nicely format retaining path. dart-lang/leak_tracker#68
    • improve formatting: Improve getting retaining path. dart-lang/leak_tracker#69
    • add constructor: Add constructor for retaining path. dart-lang/leak_tracker#72
    • fix inspector:
      • devtools: Prepare for weak references in inspector. #5880
      • first attempt: Make inspector weakly referencing the inspected objects. flutter#128095
      • second attempt: Redo make inspector weakly referencing the inspected objects. flutter#128471
      • prepare inspector in flutter to consume Diagnosticable id in APIs
        • getChildrenSummaryTree: Update getChildrenSummaryTree to handle Diagnosticable as input. flutter#128833
        • getProperties: Update getProperties to handle Diagnosticable as input. flutter#128897
        • other APIs: Accept Diagnosticable as input in inspector API. flutter#128962
      • stop passing DiagnosticsNode on DevTools side: Pass 'valueId' (Diagnosticable) instead of 'objectId' (DiagnosticsNode) to inspector API. #5918
      • stop accepting DiagnosticsNode on Flutter side: Cleanup: stop accepting DiagnosticsNode as input from DevTools. flutter#129302
      • third attempt: Next attempt to make inspector weakly referencing the inspected objects. flutter#129962
      • test that inspector does not hold objects from GC: Test that inspector does not hold objects. flutter#130102
  • investigate Next Gen UI demo crashes mobile browsers flutter#127418 (comment)
  • Turn on tracking for non-GCed
    • Figure out vm_service getInstances fails with null reference when invoked for TypeParameters dart-lang/sdk#52893
    • Investigate/file found issues: memory leak: widgets/app.dart/defaultActions holds disposed objects from being GCed flutter#130354
    • Fix inspector: Inspector should not hold callback from garbage collection. flutter#130436
    • Add troubleshooting tools and fixes for leak_tracker: Create troubleshooting helpers. dart-lang/leak_tracker#89
    • Update code: Enable not GCed leak tracking. flutter#130159
    • Update the guidance on how to debug non-gced
  • Improve performance
    • Initial improvement: go/leak-tracker-performance-challenge
    • Process tests in one file at once: Reorganize leak tracker for better performance. dart-lang/leak_tracker#106
    • request and use: Expose vm service API to give me ObjRef by object. dart-lang/sdk#53012
    • figure out Testing in VSCode attributes the failure to random tests and lines, if failure happened in tearDownAll defined in other library. flutter#131681
  • Kick off vm service if it is needed to get path: [(Service.controlWebServer(enable: true, silenceOutput: true);)
    • https://github.com/dart-lang/leak_tracker/pull/109](https://github.com/dart-lang/leak_tracker/pull/112)
    • figure out flutter issue: Cannot start vm_service for a flutter test. dart-lang/sdk#53262
  • Enable WeakReference
    • Document option to fix notGCed by converting a reference to WeakReference: https://github.com/dart-lang/leak_tracker/blob/main/doc/TROUBLESHOOT.md#general-rules
    • bump firefox: cl/551925539
    • figure out Failure to parse Firefox error flutter#131627
    • testing with Firefox: Debug mode errors in Firefox in G3. flutter#132439
  • Simplify debugging of notDisposed
    • Enable ChangeNotifier clients to dispatch event of object creation in constructor. flutter#133060
    • Users of ChangeNotifier should dispatch event of object creation in constructor. flutter#133210
    • PlatformRouteInformationProvider should dispatch creation in constructor. flutter#133353
    • FocusNode and FocusManager should dispatch creation in constructor. flutter#133352
    • _SelectableFragment should dispatch creation in constructor. flutter#133351
    • ShortcutManager should dispatch creation in constructor. [prod-leak-fix] flutter#133356
    • Make Route dispatching memory events. [prod-leak-fix] flutter#133721
  • Address breaking changes
  • Clean up notDisposed: Clean up notDisposed leaks in Flutter Framework, phase 1. dart-lang/leak_tracker#134
  • Integrate with testWidgets: Integrate leak tracking with testWidgets and stop using testWidgetsWithLeakTracking in flutter/test. flutter#135856
  • Research: Research options for disposable and leak tracker MVP #6555
• Release MVP (leak_tracker MVP #6266)
  • Design doc
  • Create alignment with Flutter team: Clean up leaks in FF in non-blocking mode and release leak tracker. #6909
  • Release
• After MVP
  • Create mixin or class Disposable
  • Add leak tracking to testWidgets, disabled by default: Enable testWidgets to track leaks. flutter#124435
    • Enable by default leak tracking for testWidgets for Flutter
    • Fix failing tests
    • Document unit testing with leak tracking
  • Publish leak tracking for tests
    • Update leak_tracker documentation with examples
    • Work with pub.get to give points for using leak_tracker
    • Create video that describes how to track leaks
  • Address Detect "neither disposed nor GCed forever"? dart-lang/leak_tracker#76
  • Enable for running apps by default in debug/profile
  • Integrate with DevTools
    • Restore integration with devtools: Remove integration with leak_tracker. #6591
    • Investigate Leak tracker crashes app dart-lang/leak_tracker#84
    • Documentation
      • Explain how to get retaining path
      • Explain how to troubleshoot failing tests
    • add DevTools link
    • Reference constants that are defined both in DevTools and Flutter (see TODOs for this issue in DevTools)
    • Configure telemetry in DevTools
    • Set limit for tracked objects
    • Create UI in DevTools
    • Publish flutter_leak_tracker?
    • Ignore inspector, when DevTools is connected
  • Adopt for:
    • Gallery
    • money app - go/leaktracker-for-money-app
    • Analyzer - go/dart-leak-tracking-for-analyzer
    • DevTools
    • touch app - go/dart-leak-tracking-for-touch-app
    • mulligan (CM)
    • profiler
  • Extend leak tracker
    • Release mode: Enable leak tracking for release mode. dart-lang/leak_tracker#25
    • Leak tracking for web
    • Handle double disposal: Make double disposal and double creation visible. dart-lang/leak_tracker#118

[polina-c]

not-GCed leaks:

• EditableTextState for Gallery: Flutter framework is leaking in debug mode for gallery app. flutter#107338
• Navigation for DevTools: Logging screen is leaking with not-GCed _LoggingScreenState, in profile mode. #4416
• Example of detection of passed context to closure: https://github.com/polina-c/spikes/tree/master/not_gced_leak

not-disposed leaks:

• Gallery:
  • Dispose KeepAliveHandle. [prod-leak-fix] flutter#108227
  • Dispose _TextSpanEditingController. [prod-leak-fix] flutter#108228
  • Dispose painters [prod-leak-fix] flutter#108233
  • Dispose scaffold drawers. flutter#108234
• Money app:
  • Dispose widgets to stop leaks. flutter#108193

[polina-c]

Options for Flutter specific leak tracker:

1. package:leak_tracker_flutter published in tools.dart.dev, and banking on 'flutter' in the name being clarifying enough
2. keeping leak_tracker_flutter in the same mono-repo, but publishing it out of the flutter.dev publisher
3. putting the code in the flutter/packages mono-repo and using the flutter.dev publisher

@devoncarew leans towards the 2nd and 3rd options

[stuartmorgan]

2. keeping leak_tracker_flutter in the same mono-repo, but publishing it out of the flutter.dev publisher

It appears we did this; is there discussion/context somewhere on why that option was chosen? AFAIK this is the only flutter.dev-published package that is not in flutter/packages, which makes this pretty confusing.

[polina-c]

2. keeping leak_tracker_flutter in the same mono-repo, but publishing it out of the flutter.dev publisher

It appears we did this; is there discussion/context somewhere on why that option was chosen? AFAIK this is the only flutter.dev-published package that is not in flutter/packages, which makes this pretty confusing.

Here is the issue with discussion: dart-lang/leak_tracker#52 (comment)