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

Memory issue when sharing CancelToken between posts for IO implementation #2111

Open
kunstmusik opened this issue Feb 16, 2024 · 6 comments
Open
Labels
b: regression Worked before but not now p: dio Targeting `dio` package platform: io s: bug Something isn't working

Comments

@kunstmusik
Copy link

Package

dio

Version

5.4.0

Operating-System

Android

Output of flutter doctor -v

[✓] Flutter (Channel stable, 3.13.6, on macOS 14.2.1 23C71 darwin-arm64, locale en-US)
    • Flutter version 3.13.6 on channel stable at /Users/stevenyi/fvm/versions/3.13.6
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision ead455963c (5 months ago), 2023-09-26 18:28:17 -0700
    • Engine revision a794cf2681
    • Dart version 3.1.3
    • DevTools version 2.25.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/stevenyi/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)
    • All Android licenses accepted.

[!] Xcode - develop for iOS and macOS (Xcode 15.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15C500b
    ✗ Unable to get list of installed Simulator runtimes.
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)

[✓] IntelliJ IDEA Ultimate Edition (version 2023.2)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.86.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.82.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 14.2.1 23C71 darwin-arm64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 121.0.6167.184

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

Dart Version

3.1.3

Steps to Reproduce

  1. Create a CancelToken
  2. Make multiiple requests passing in the same CancelToken

Expected Result

No out of memory issues on Android.

Actual Result

Out of memory issues occur.

Dio's use of cancelFuture in dio/lib/src/adapters/io_adapter.dart differs from how it is used in browser_adapter.dart and exhibits different behavior. In IO, it captures on request in the callback to whenComplete in _fetch, which does not happen in the browser implementation.

This took us some time to discover as a memory leak issue. It would be great if this could be fixed, otherwise would love to see this documented so that future users will be aware of this issue.

@kunstmusik kunstmusik added h: need triage This issue needs to be categorized s: bug Something isn't working labels Feb 16, 2024
@AlexV525
Copy link
Member

Could you attach the minimal reproducible example and some analysis graphs or details?

@AlexV525 AlexV525 added the h: need more info Further information is requested label Mar 3, 2024
@zambetpentru

This comment was marked as off-topic.

@AlexV525
Copy link
Member

@kuhnroyal I have been thinking about the cause of this issue for the last few days, it could be caused by referencing the resources of the request in callers like .whenComplete so they will only get released once the CancelToken canceled, but it doesn't, so memory leaks. Any ideas?

@AlexV525
Copy link
Member

If yes, we might add a global map to cache the necessary instances for the corresponding cancel token.

@kuhnroyal
Copy link
Member

I tried to enable the new leak tracking but didn't get very far yet...

@kunstmusik
Copy link
Author

@AlexV525 I haven't had time to followup, but yes, as mentioned in my first message, we found that the .whenComplete callback captures over the Request:

      if (cancelFuture != null) {
        cancelFuture.whenComplete(() => request.abort());
      }

and whenComplete() also adds additional handlers each time it is called. So if a CancelToken is passed in to multiple fetches, it will keep accruing whenComplete handlers that all capture over individual requests.

@AlexV525 AlexV525 added p: dio Targeting `dio` package b: regression Worked before but not now platform: io and removed h: need more info Further information is requested h: need triage This issue needs to be categorized labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b: regression Worked before but not now p: dio Targeting `dio` package platform: io s: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants