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

Conversation

@zsunkun
Copy link
Contributor

@zsunkun zsunkun commented Aug 20, 2021

Each Engine should use its own io_manager when spawn new engine. The current situation is that when a new Engine is spawned from an old Engine, the new Engine will use the io_manager of the old Engine. the new Engine will be crash when the old Engine is destroyed.
For example:
device-2021-08-20-195426
In this case, two engines are used to display GIF images on the same page, when I remove the top flutterView and destroy the engine, the bottomEngine will continue to decode pictures, but it will be crash because io_manager is null
image

In addition, when I modified the code, I found that ImageDeocoder also had the same problem. In other words, there will be similar crash in certain situations ,please take a look

@google-cla
Copy link

google-cla bot commented Aug 20, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Aug 20, 2021
@zsunkun zsunkun force-pushed the use_separate_io_manager_sunkun branch from 465051b to 100797d Compare August 20, 2021 12:49
@google-cla google-cla bot added cla: yes and removed cla: no labels Aug 20, 2021
@gaaclarke gaaclarke self-requested a review August 23, 2021 16:51
@eggfly
Copy link
Member

eggfly commented Aug 23, 2021

@gaaclargar @blasten @iskakaushis A PR of my teammate, please take a review~ thanks

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

Hi zsunkun, good detective work. This PR is going to need tests. I recommend writing tests that recreate the crash (without your change).

@chinmaygarde
Copy link
Member

Agreed, the test to add would go in the shell_unittests harness. Perhaps a variant of TEST_F(ShellTest, Spawn)?

@chinmaygarde
Copy link
Member

Any updates?

@chinmaygarde
Copy link
Member

Closing this as stale. Please re-open once this is good for another review. Thanks.

@eggfly
Copy link
Member

eggfly commented Nov 24, 2021

This PR takes place of it #29915

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants