Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@0xZOne
Copy link
Member

@0xZOne 0xZOne commented Sep 7, 2021

Try to fix issue: #89408

Consider sharing engine scenario, when the previous FlutterView detaches from the engine, the overlay Surface cached in the PlatformViewController destroyed, and the corresponding surface in SurfacePool is still alive and used for the current FlutterView. This inconsistency causes an assertion crash.

To make SurfacePool work for the shared engine scenario, we completely here hand over the responsibility of destroying the overlay surface cached by PlatformViewsController to SurfacePool.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@chinmaygarde
Copy link
Member

cc @blasten

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems more of a general issue. shouldn't each engine have its own SurfacePool? @gaaclarke

@gaaclarke
Copy link
Member

I don't see a reason why lightweight engines would need their own SurfacePool. @xster implemented this and systematically looked at all the resources resident in memory and shared them when possible. Lightweight engines share threads so they will be communicating with this pool serially, so there is no need to have multiple pools.

@0xZOne
Copy link
Member Author

0xZOne commented Sep 18, 2021

This seems more of a general issue. shouldn't each engine have its own SurfacePool? @gaaclarke

Here is the case where multiple Flutter Views reuse a single engine.

@chinmaygarde
Copy link
Member

@blasten & @gaaclarke are you able to suggest a resolution to @0xZOne s issue without needing to creating separate pools? @xster is probably no longer owning these components. So we have to decide how to proceed with issues raised here.

@zanderso
Copy link
Member

zanderso commented Oct 7, 2021

@blasten @gaaclarke From PR triage, it looks like the status of this PR is an open question for you from @chinmaygarde . PTAL, thanks!

@chinmaygarde
Copy link
Member

Any guidance here? If not, a short term solution to fix the crash would be to go with the separate pools.

@0xZOne
Copy link
Member Author

0xZOne commented Oct 22, 2021

Fixed by #28894

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants