Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect "neither disposed nor GCed forever"? #76

Open
fzyzcjy opened this issue Jun 3, 2023 · 1 comment
Open

Detect "neither disposed nor GCed forever"? #76

fzyzcjy opened this issue Jun 3, 2023 · 1 comment
Labels
P2 A bug or feature request we're likely to work on Size:S days

Comments

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Jun 3, 2023

Please upvote and comment on the issue if interested.

Why this is needed

Consider the common scenario: I have a ui.Image, a native audio player, or whatever heavy resource. Then, I want to ensure that, when the widget test is end (or even better, when the page is left in app), there are zero heavy resources there. This check can reveal bugs of the type "neither disposed nor GCed forever".

When will this happen in reality? Firstly, I have seen it in my app. Secondly, generally speaking, this can happen when users forget to call dispose + the object has a retaining path thus not GCed as well.

Will the proposed check cause false positives? I think it may be alleviated by using a whitelist.

Proposal

A naive implementation may look like this:

void testWidgetsWithLeakTracking(body) {
  body();
  expect(getAllObjectsThatCallsCreationButNotDisposal(), isEmpty);
}

In other words, if a object calls dispatchObjectCreated but never dispatchObjectDisposed , then warn it.

For a minimal implementation, the code can be seen in the last section.

Reproduction sample

If I understand correctly how to use this package, here are reproduction sample. (You need to copy-paste the https://github.com/flutter/flutter/blob/60a87d079843fa1a3498d7f3263b076a23074221/packages/flutter/test/foundation/leak_tracking.dart file before using the following code.)

Here, I simulated two scenarios, one using a widget, and another using direct allocation. (Indeed in the first scenario it may be GCed but I am not very sure.) I expect two tests to fail using the detector, but they both passed happily.

import 'package:common_flutter_dev/src/leak_tracking.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
  group('simple play with leak tracker', () {
    testWidgetsWithLeakTracking('StatefulWidget containing disposable objects', (tester) async {
      await tester.pumpWidget(const _SimpleWidget());
    });

    testWidgetsWithLeakTracking('objects that live forever', (tester) async {
      final controller = ScrollController()..addListener(_dummyListener);
      _notGcedStorage.add(controller);
      // _notGcedStorage.remove(controller); // comment it out to simulate leak
    });
  });
}

final _notGcedStorage = <Object>[];

class _SimpleWidget extends StatefulWidget {
  const _SimpleWidget();

  @override
  State<_SimpleWidget> createState() => _SimpleWidgetState();
}

class _SimpleWidgetState extends State<_SimpleWidget> {
  late final ScrollController controller;

  @override
  void initState() {
    super.initState();
    controller = ScrollController()..addListener(_dummyListener);
  }

  @override
  void dispose() {
    // controller.dispose(); // comment it out to simulate leak
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return const Placeholder();
  }
}

void _dummyListener() {}

Minimal implementation code

Here is a minimal implementation I used to detect non-disposed and non-gced ui.Image.

import 'dart:async';
import 'dart:ui' as ui;

import 'package:collection/collection.dart';
import 'package:flutter/foundation.dart';
import 'package:front_log/front_log.dart';

abstract class ObjectRetainChecker {
  static const _kTag = 'ObjectRetainChecker';

  @protected
  final activeObjects = Set<Object>.identity();

  ObjectRetainChecker() {
    if (kReleaseMode) throw Exception('Are you sure you want to use $_kTag in release mode?');
    MemoryAllocations.instance.addListener(_handleMemoryAllocations);
  }

  void _handleMemoryAllocations(ObjectEvent event) {
    if (!isInterestObject(event.object)) return;

    if (event is ObjectCreated) {
      activeObjects.add(event.object);
    } else if (event is ObjectDisposed) {
      activeObjects.remove(event.object);
    }
  }

  @protected
  bool isInterestObject(Object object);
}

class UiImageRetainChecker extends ObjectRetainChecker {
  static const _kTag = 'UiImageRetainChecker';

  UiImageRetainChecker() {
    Timer.periodic(const Duration(seconds: 10), (_) => dumpInfo());
  }

  void dumpInfo() {
    final images = activeObjects.cast<ui.Image>();
    final imageSizeBytes = images.map((image) => image.sizeBytes).toList();
    Log.i(
        _kTag,
        'Active `ui.Image`s: '
        'total bytes (may have duplicates) = ${_formatBytes(imageSizeBytes.sum)} '
        '#image = ${imageSizeBytes.length} '
        'detailed bytes = ${imageSizeBytes.map(_formatBytes).toList()}',
        self: this);
  }

  String _formatBytes(int bytes) => '${(bytes / 1000000).toStringAsFixed(3)}MB';

  @override
  bool isInterestObject(Object object) => object is ui.Image;
}

extension on ui.Image {
  // https://github.com/flutter/flutter/issues/128163
  int get sizeBytes => (width * height * 4 * (4 / 3)).toInt();
}
@fzyzcjy fzyzcjy changed the title Is it possible to detect "neither disposed nor GCed forever"? (Willing to PR) Is it possible to detect "neither disposed nor GCed forever"? Jun 3, 2023
@fzyzcjy fzyzcjy changed the title (Willing to PR) Is it possible to detect "neither disposed nor GCed forever"? Is it possible to detect "neither disposed nor GCed forever"? Jun 3, 2023
@polina-c
Copy link
Contributor

Thanks for proposal. Added to the project.

@polina-c polina-c added the P2 A bug or feature request we're likely to work on label Aug 14, 2023
@polina-c polina-c changed the title Is it possible to detect "neither disposed nor GCed forever"? Detect "neither disposed nor GCed forever"? Aug 14, 2023
@polina-c polina-c added the Size:S days label Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on Size:S days
Projects
None yet
Development

No branches or pull requests

2 participants