This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Revert "Provide dart vm initalize isolate callback so that children i… #12327
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…solates belong to parent's isolate group. (flutter#9888)" This reverts commit e6620dd.
chinmaygarde
approved these changes
Sep 17, 2019
|
lgtm. Would it have been possible to get --system-verbose-logs from running flutter engine instance to see what isolates get initialized/shutdown/cleaned up? |
|
@aam i haven't tried that. I will work on getting you those once the roller is unblocked. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 18, 2019
…ildren isolates belong to parent's isolate group. (flutter#9888)" (flutter/engine#12327)
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Sep 18, 2019
[email protected]:flutter/engine.git/compare/6a96417416b4...36be89d git log 6a96417..36be89d --no-merges --oneline 2019-09-17 [email protected] Added javadoc comments to FlutterActivity and FlutterFragmentActivity. (flutter/engine#12328) 2019-09-17 [email protected] Roll src/third_party/dart 7505b3a5f0..f22f62c85d (5 commits) 2019-09-17 [email protected] Revert "Provide dart vm initalize isolate callback so that children isolates belong to parent's isolate group. (#9888)" (flutter/engine#12327) 2019-09-17 [email protected] [flutter] Remove old A11y API's. (flutter/engine#12308) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
|
@chinmaygarde We would really like to have full end2end tests for cases like this. Does flutter dashboard / flutter CI builders have such end2end test functionality? If so, could you point us e.g. to an existing test? |
|
If I understand correctly flutter gallery example(that is used in many devicelab tests) does use child isolate to load licenses(https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/services/asset_bundle.dart#L75), so basic flow of spawning child isolate, having it terminated should be covered by that. |
|
We should have custom end2end tests which spawn new isolates, do some work in them, make the isolate die and ensure everything worked as expected. Similarly we should have such a test, but also use hot reload. |
Inconnu08
pushed a commit
to Inconnu08/flutter
that referenced
this pull request
Sep 30, 2019
[email protected]:flutter/engine.git/compare/6a96417416b4...36be89d git log 6a96417..36be89d --no-merges --oneline 2019-09-17 [email protected] Added javadoc comments to FlutterActivity and FlutterFragmentActivity. (flutter/engine#12328) 2019-09-17 [email protected] Roll src/third_party/dart 7505b3a5f0..f22f62c85d (5 commits) 2019-09-17 [email protected] Revert "Provide dart vm initalize isolate callback so that children isolates belong to parent's isolate group. (flutter#9888)" (flutter/engine#12327) 2019-09-17 [email protected] [flutter] Remove old A11y API's. (flutter/engine#12308) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
aam
added a commit
to aam/engine
that referenced
this pull request
Oct 31, 2019
…ildren isolates belong to parent's isolate group. (flutter#9888)" (flutter#12327)" This reverts commit 23835f5.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…solates belong to parent's isolate group. (#9888)"
The earlier behavior was that the isolate shutdown callback used to be associated with a single isolate. We take this to mean that when the root isolate shuts down, we tear down the entire Flutter shell. Now, the callback seems to be invoked when any isolate in the isolate group is killed. This in turn makes us kill the Flutter shell when any isolate in the group dies.
Hypothesis is that when a child isolate shuts down, we call the shutdown callback on the root isolate. Which results in the whole flutter runner shutting down (by killing the Engine). This in turn results in the behavior as described in the bug fxb/36670.
This reverts commit e6620dd.