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 leak when using PlatformView [IOS] #24714

Closed
An-uking opened this issue Nov 25, 2018 · 17 comments · Fixed by flutter/engine#7919
Closed

Memory leak when using PlatformView [IOS] #24714

An-uking opened this issue Nov 25, 2018 · 17 comments · Fixed by flutter/engine#7919
Labels
a: platform-views Embedding Android/iOS views in Flutter apps c: performance Relates to speed or footprint issues (see "perf:" labels) package flutter/packages repository. See also p: labels.

Comments

@An-uking
Copy link

An-uking commented Nov 25, 2018

Steps to Reproduce

I write a RTMP Video plugin for flutter  
I open the video page and  close it .
Keep this operation, more than a dozen times, memory leak

NSLog

PiliPlayer:video player plugin register
PiliPlayer:playerfactory init

open video page
PiliPlayer:viewId -- 0
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 1
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 2
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 3
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 4
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 5
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 6
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 7
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 8
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 9
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 10
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 11
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 12
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 13
PiliPlayer:player create

close page
PiliPlayer:player dispose
PiliPlayer:playerContainerView dispose

open video page
PiliPlayer:viewId -- 14
PiliPlayer:player create

Lost connection to device.
Exited (sigterm)
[✓] Flutter (Channel beta, v0.11.9, on Mac OS X 10.13.6 17G3025, locale
    zh-Hans-CN)
[✓] Android toolchain - develop for Android devices (Android SDK 28.0.3)
[✓] iOS toolchain - develop for iOS devices (Xcode 10.1)
[✓] Android Studio (version 3.2)
[✓] VS Code (version 1.29.1)
[✓] Connected device (1 available)

@zoechi
Copy link
Contributor

zoechi commented Nov 26, 2018

Is this with video player logic active or just opening/closing PlatformViews?

@zoechi zoechi added waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds c: performance Relates to speed or footprint issues (see "perf:" labels) a: platform-views Embedding Android/iOS views in Flutter apps labels Nov 26, 2018
@qypengzai
Copy link

I met same case. when I rebuild the render tree that platform view has inserted, the native method 'create' will be called. However, It is not my expectation all the time. for example, sometimes I only wanna change my platform layout.

@qypengzai
Copy link

platform -> platform view

@An-uking
Copy link
Author

An-uking commented Dec 2, 2018

Is this with video player logic active or just opening/closing PlatformViews?

qq 20181202164354

qq 20181202164424

how to release platformview?

@no-response no-response bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Dec 2, 2018
@An-uking
Copy link
Author

An-uking commented Dec 2, 2018

@zoechi

the plugin's code is written like flutter's webview_flutter plugin

@zoechi zoechi added this to the Goals milestone Dec 3, 2018
@zoechi zoechi added the plugin label Dec 3, 2018
@amirh
Copy link
Contributor

amirh commented Dec 28, 2018

I don't think this is a Flutter bug, for comparison here's what the memory graph looks like when I open and close the "scrolling map" page in the Google Maps demo (which have 2 Google Maps in it):

screen shot 2018-12-28 at 9 46 16 am

(the spikes are when the maps page is open, and you can see it goes down when the page is closed)

I would guess that the RTMP video plugin is causing a leak.
Note that a sneaky pitfall would be to keep a strong reference to your UIView from the method channel handler (which will result in a reference cycle), to avoid this e.g in the maps plugin I pass a weak reference to the method channel handler: (https://github.com/flutter/plugins/blob/860ebfe984d4b1c6cd83f6c03b84278ca8db7d62/packages/google_maps_flutter/ios/Classes/GoogleMapController.m#L70)

@qypengzai note that the lifecycle of the underlying platform view is the same as the lifecycle of the UiKitView widget's state. If you want to e.g move it in the widget tree you need to ensure you don't lose state by setting a key on the widget.

@amirh amirh closed this as completed Dec 28, 2018
@An-uking
Copy link
Author

@amirh Thanks for replying, I have constructed an IJKPlayer video plugin in a textured way to support live streaming and on-demand streaming. Through the way of texture, I have not encountered the above problem.

@Natoto
Copy link

Natoto commented Feb 21, 2019

Finally found a solution ✌️
https://juejin.im/post/5c6e6dd5f265da2dcf62821f

@ashawn
Copy link

ashawn commented Feb 21, 2019

Finally found a solution ✌️
https://juejin.im/post/5c6e6dd5f265da2dcf62821f

IOSGLRenderTarget::~IOSGLRenderTarget() {
  [EAGLContext setCurrentContext:context_];
  FML_DCHECK(glGetError() == GL_NO_ERROR);

  // Deletes on GL_NONEs are ignored
  glDeleteFramebuffers(1, &framebuffer_);
  glDeleteRenderbuffers(1, &colorbuffer_);

  FML_DCHECK(glGetError() == GL_NO_ERROR);
}

@amirh
Copy link
Contributor

amirh commented Feb 21, 2019

Thanks for the investigation and fix @ashawn!

In general I encourage you to file issues, or re-open closed issues if you think it's a mistake and the issue is not resolved.

For this fix(and any other fix) I'd encourage you to send a PR with the fix to the Flutter Engine so everyone can benefit.

Thanks!

@ashawn
Copy link

ashawn commented Feb 22, 2019

Thanks for the investigation and fix @ashawn!

In general I encourage you to file issues, or re-open closed issues if you think it's a mistake and the issue is not resolved.

For this fix(and any other fix) I'd encourage you to send a PR with the fix to the Flutter Engine so everyone can benefit.

Thanks!

Hi I have sent a PR with the fix to the Flutter Engine, flutter/engine#7919
Thanks for your encouragement!

@An-uking
Copy link
Author

@ashawn thank you ! I build Flutter_IJKPlayer plugin by Texture ,now,not use PlatformView

engine-flutter-autoroll added a commit that referenced this issue Feb 22, 2019
flutter/engine@33bb91c...f80928a

git log 33bb91c..f80928a --no-merges --oneline
f80928a Roll src/third_party/skia fdd15284affe..ee1c8a733e5b (15 commits) (flutter/engine#7924)
93f339f fix Memory leak when using PlatformView [IOS] #24714 (flutter/engine#7919)
2d33e77 Roll src/third_party/skia 969659dbb313..fdd15284affe (2 commits) (flutter/engine#7922)
6d4edf2 Roll src/third_party/skia 9431161ca973..969659dbb313 (3 commits) (flutter/engine#7921)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (amirha@google.com), and stop
the roller if necessary.
Natoto added a commit to Natoto/engine that referenced this issue Mar 5, 2019
remove
 //fix platformview memory leak
flutter/flutter#24714
  [EAGLContext setCurrentContext:context_.get()];
@darkmarthur
Copy link

darkmarthur commented Jun 28, 2019

I don't think this is a Flutter bug, for comparison here's what the memory graph looks like when I open and close the "scrolling map" page in the Google Maps demo (which have 2 Google Maps in it):

screen shot 2018-12-28 at 9 46 16 am

(the spikes are when the maps page is open, and you can see it goes down when the page is closed)

I would guess that the RTMP video plugin is causing a leak.
Note that a sneaky pitfall would be to keep a strong reference to your UIView from the method channel handler (which will result in a reference cycle), to avoid this e.g in the maps plugin I pass a weak reference to the method channel handler: (https://github.com/flutter/plugins/blob/860ebfe984d4b1c6cd83f6c03b84278ca8db7d62/packages/google_maps_flutter/ios/Classes/GoogleMapController.m#L70)

@qypengzai note that the lifecycle of the underlying platform view is the same as the lifecycle of the UiKitView widget's state. If you want to e.g move it in the widget tree you need to ensure you don't lose state by setting a key on the widget.

@amirh @zoechi @mravn-google @timtraversy
Hi Amirh, I noticed that you are a very active engineer on the google_maps_flutter repo this issue should be reopen

Problem:
I have been facing a critical issue with the GoogleMap plugin, the problem is on iOS devices, when the GoogleMap widget is RENDERED (please note that the problem is not when instantiating the object) it appends a lot of memory to the current app, when the map is disposed, that memory allocated never goes away, it keeps growing as the Map is called to be shown.

Why it is a critical issue:
This is a critical issue because on iOS when the app reaches approximately 1gb the OS closes the app unexpectedly, and its a blocker to launch our app to production.

Replicate:
Use the memory monitor of XCode, not the dart tools,
This is not replicable on the emulator, only on real iOS devices, you can use the google_maps_flutter example to test.

Request:
Please let us know if you can replicate the issue just to be sure that this is on your scope

Evidence:
Video
https://drive.google.com/open?id=1c78VJhBYsKc8HRX-8_RiR56J1LHnoWvJ

@amirh
Copy link
Contributor

amirh commented Jul 17, 2019

@darkmarthur can you file a new issue for this, there was a memory leak in destruction of platform views that was fixed a few months ago not sure if you got it or not, can you include the output for flutter doctor -v on the new issue? a minimal app to reproduce the issue will also be useful.

Thanks!

@nuptdzs
Copy link

nuptdzs commented Dec 19, 2019

@amirh
hi,I meet this problem again.is there any changes reproduce this problem.
with IOS ,I use webview_flutter plugin,I repeatedly enter and exit this page .The memory will grow all the time, and in the leaks tool of Xcode, we can see that a variable is not released after exiting, as shown in the figure below, it seems that some other changes to GL cause this problem.
image
image

`[✓] Flutter (Channel stable, v1.12.13+hotfix.5, on Mac OS X 10.14.6 18G87, locale zh-Hans-CN)

[!] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
! Some Android licenses not accepted. To resolve this, run: flutter doctor --android-licenses
[✓] Xcode - develop for iOS and macOS (Xcode 11.3)
[✓] Android Studio (version 3.5)
[✓] VS Code (version 1.38.1)
[!] Proxy Configuration
! NO_PROXY is not set
[✓] Connected device (1 available)
`

Can you try to analyze this problem, and look forward to your answer. This problem will cause our application memory to grow until it crashes, hoping to give us a solution.

@amirh
Copy link
Contributor

amirh commented Dec 30, 2019

Thanks @nuptdzs , I filed #47992 to track the newly reported issue

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
@flutter-triage-bot flutter-triage-bot bot added the package flutter/packages repository. See also p: labels. label Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: platform-views Embedding Android/iOS views in Flutter apps c: performance Relates to speed or footprint issues (see "perf:" labels) package flutter/packages repository. See also p: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants