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 · 12 comments

Comments

Projects
None yet
6 participants
@An-uking
Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2018

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

@qypengzai

This comment has been minimized.

Copy link

commented Nov 29, 2018

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

This comment has been minimized.

Copy link

commented Nov 29, 2018

platform -> platform view

@An-uking

This comment has been minimized.

Copy link
Author

commented Dec 2, 2018

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

qq 20181202164354

qq 20181202164424

how to release platformview?

@An-uking

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

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 Dec 28, 2018

@An-uking

This comment has been minimized.

Copy link
Author

commented Dec 28, 2018

@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

This comment has been minimized.

Copy link

commented Feb 21, 2019

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

@ashawn

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Author

commented Feb 22, 2019

@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

Roll engine 33bb91c..f80928a (4 commits) (#28353)
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

update ios_glrender_target
remove
 //fix platformview memory leak
flutter/flutter#24714
  [EAGLContext setCurrentContext:context_.get()];

KevinG2011 pushed a commit to pepper-ios/flutter that referenced this issue May 9, 2019

Roll engine 33bb91c..f80928a (4 commits) (flutter#28353)
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] flutter#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.