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

Unable to release FlutterViewController even when there is nothing referencing it. #21347

Closed
nightwolf-chen opened this issue Sep 3, 2018 · 14 comments
Assignees
Labels
a: existing-apps Integration with existing apps via the add-to-app flow customer: alibaba e: embedder Users of the Embedder API platform-ios iOS applications specifically
Milestone

Comments

@nightwolf-chen
Copy link

nightwolf-chen commented Sep 3, 2018

We‘ve embed Flutter into our existing App. The problem is that we want to release FlutterViewController when there are no Flutter Pages to save some memory. But we found that FlutterViewController can't be released even there are no references to it.

I've debug into Flutter Engine and found that there are some retain circles between FlutterChannels and FlutterViewController. I tried to break the retain circles and released FlutterViewController successfully. But there are some Skia Images leaked during the release of FlutterViewController.

I was wondering is there any official support for FlutterViewController releasing and rebuilding without those memory leak problems.

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel xy_beta_v0.5.6, v0.5.6-pre.112, on Mac OS X 10.13.5 17F77, locale zh-Hans-CN)
[✓] Android toolchain - develop for Android devices (Android SDK 27.0.3)
[✓] iOS toolchain - develop for iOS devices (Xcode 9.4.1)
[✓] Android Studio (version 3.1)
[✓] VS Code (version 1.25.1)
[✓] Connected devices (1 available)

• No issues found!
@zoechi zoechi added the a: existing-apps Integration with existing apps via the add-to-app flow label Sep 3, 2018
@xster
Copy link
Member

xster commented Sep 18, 2018

cc @jamesderlin

@seanWongs
Copy link

Is there any progress?

@jamesderlin
Copy link
Contributor

cc @dnfield

@nightwolf-chen
Copy link
Author

flutter/engine#6464 This should be related to the retain circles.

@dnfield
Copy link
Contributor

dnfield commented Oct 9, 2018

flutter/engine#6447 should make this more doable. I'm curious about the Skia images leaking though, do you have a reproduction for that by chance?

@dnfield dnfield added platform-ios iOS applications specifically e: embedder Users of the Embedder API labels Oct 9, 2018
@dnfield dnfield self-assigned this Oct 9, 2018
@nightwolf-chen
Copy link
Author

nightwolf-chen commented Oct 11, 2018

It just happens when you dealloc shell inside FlutterViewController. When I tried to release shell, the Skia Objects unref operation causes a crash for null pointer exceptions. When I tried to avoid the crashes by skipping the unref operation the leaks happened. My guess is that the Skia images have to be released on GPU thread and there were some threads synchronization problems. It could be related to code blow.

shell deconstructor:

Shell::~Shell() {
  PersistentCache::GetCacheForProcess()->RemoveWorkerTaskRunner(
      task_runners_.GetIOTaskRunner());

  if (auto vm = blink::DartVM::ForProcessIfInitialized()) {
    vm->GetServiceProtocol().RemoveHandler(this);
  }

  fml::AutoResetWaitableEvent ui_latch, gpu_latch, platform_latch, io_latch;

  fml::TaskRunner::RunNowOrPostTask(
      task_runners_.GetUITaskRunner(),
      fml::MakeCopyable([engine = std::move(engine_), &ui_latch]() mutable {
        engine.reset();
        ui_latch.Signal();
      }));
  ui_latch.Wait();

  fml::TaskRunner::RunNowOrPostTask(
      task_runners_.GetGPUTaskRunner(),
      fml::MakeCopyable(
          [rasterizer = std::move(rasterizer_), &gpu_latch]() mutable {
            rasterizer.reset();
            gpu_latch.Signal();
          }));
  gpu_latch.Wait();

  fml::TaskRunner::RunNowOrPostTask(
      task_runners_.GetIOTaskRunner(),
      fml::MakeCopyable([io_manager = std::move(io_manager_),
                         platform_view = platform_view_.get(),
                         &io_latch]() mutable {
        io_manager.reset();
        if (platform_view) {
          platform_view->ReleaseResourceContext();
        }
        io_latch.Signal();
      }));

  io_latch.Wait();

  // The platform view must go last because it may be holding onto platform side
  // counterparts to resources owned by subsystems running on other threads. For
  // example, the NSOpenGLContext on the Mac.
  fml::TaskRunner::RunNowOrPostTask(
      task_runners_.GetPlatformTaskRunner(),
      fml::MakeCopyable([platform_view = std::move(platform_view_),
                         &platform_latch]() mutable {
        platform_view.reset();
        platform_latch.Signal();
      }));
  platform_latch.Wait();
}

unref code:

void SkiaUnrefQueue::Unref(SkRefCnt* object) {
  std::lock_guard<std::mutex> lock(mutex_);
  objects_.push_back(object);
  if (!drain_pending_) {
    drain_pending_ = true;
    task_runner_->PostDelayedTask(
        [strong = fml::Ref(this)]() { strong->Drain(); }, drain_delay_);
  }
}

void SkiaUnrefQueue::Drain() {
  std::deque<SkRefCnt*> skia_objects;
  {
    std::lock_guard<std::mutex> lock(mutex_);
    objects_.swap(skia_objects);
    drain_pending_ = false;
  }

  for (SkRefCnt* skia_object : skia_objects) {
    skia_object->unref();
  }
}

@nightwolf-chen
Copy link
Author

Please notice this problem was on 0.5.6 beta version.

@dnfield
Copy link
Contributor

dnfield commented Oct 16, 2018

Can you try with the code in flutter/engine#6447 and see if you're able to successfully release the view controller without memory leaks? I don't know if I'm doing exactly what you did, but I'm pretty sure it should work with that patch.

@dnfield dnfield added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 16, 2018
@dnfield dnfield added this to the bucket10 milestone Oct 16, 2018
@nightwolf-chen
Copy link
Author

@dnfield I didn't see the same problems so far. I will continue investigate. For now I should just close the issue for this particular problem. Thanks!

@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 Oct 25, 2018
@firetheworld
Copy link

firetheworld commented Dec 14, 2018

@nightwolf-chen @dnfield I have this problem in flutter release 1.0
I follow this page step by step
Only different is that I use navigation push the FlutterViewController, not present.
After several times push and pop operations, memory has increased to 300+M
I'm using iPhone 6 with iOS 11.3 for test
And here is my flutter doctor result:

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, v1.0.0, on Mac OS X 10.13.6 17G65, locale
    zh-Hans-CN)
[!] Android toolchain - develop for Android devices (Android SDK 25.0.3)
    ✗ Android license status unknown.
[✓] iOS toolchain - develop for iOS devices (Xcode 10.0)
[✓] Android Studio (version 3.2)
[!] IntelliJ IDEA Ultimate Edition (version 2017.1.2)
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    ✗ Dart plugin not installed; this adds Dart specific functionality.
[!] VS Code (version 1.30.0)
[✓] Connected device (2 available)

@zoechi
Copy link
Contributor

zoechi commented Dec 14, 2018

@firetheworld sounds like #25255

@Natoto
Copy link

Natoto commented Feb 15, 2019

@krishnakirana
Copy link

Call flutterEngine?.destroyContext() when you want release flutterViewController

@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 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: existing-apps Integration with existing apps via the add-to-app flow customer: alibaba e: embedder Users of the Embedder API platform-ios iOS applications specifically
Projects
None yet
Development

No branches or pull requests

9 participants