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

Test child isolates are terminated when root is shutdown - #13048

Merged
GaryQian merged 3 commits into
flutter-team-archive:masterfrom
GaryQian:isolate
Oct 10, 2019
Merged

GaryQian merged 3 commits into
flutter-team-archive:masterfrom
GaryQian:isolate

Conversation

@GaryQian

@GaryQian GaryQian commented Oct 9, 2019

Copy link
Copy Markdown
Contributor

@chinmaygarde chinmaygarde left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some nits about the unused object. But otherwise LGTM. Thanks.

Comment thread runtime/dart_isolate_unittests.cc Outdated
TEST_F(DartIsolateTest, RootIsolateShutdownStopsChildIsolates) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
fml::CountDownLatch latch(9);
fml::CountDownLatch shutdown_latch(4);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this latch redundant? You are not going to count down on the latch above 9 times unless 4 of them are also from the shutdown? Your call on if you think is clearer though.

Comment thread runtime/dart_isolate_unittests.cc Outdated
};
auto vm_ref = DartVMRef::Create(settings);
auto vm_data = vm_ref.GetVMData();
TaskRunners task_runners(GetCurrentTestName(), //

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These task runners are not necessary and not being used at all.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You created a new thread for it explicitly below.

@GaryQian
GaryQian merged commit e96c740 into flutter-team-archive:master Oct 10, 2019
chinmaygarde added a commit that referenced this pull request Oct 10, 2019
chinmaygarde added a commit that referenced this pull request Oct 10, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Oct 10, 2019
[email protected]:flutter/engine.git/compare/5162413111b8...cef6751

git log 5162413..cef6751 --no-merges --oneline
2019-10-10 [email protected] Revert "Test child isolates are terminated when root is shutdown (#13048)" (flutter-team-archive/engine#13067)
2019-10-10 [email protected] Test child isolates are terminated when root is shutdown (flutter-team-archive/engine#13048)
2019-10-10 [email protected] Allow embedders to specify arbitrary data to the isolate on launch. (flutter-team-archive/engine#13047)


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
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
[email protected]:flutter/engine.git/compare/5162413111b8...cef6751

git log 5162413..cef6751 --no-merges --oneline
2019-10-10 [email protected] Revert "Test child isolates are terminated when root is shutdown (flutter#13048)" (flutter-team-archive/engine#13067)
2019-10-10 [email protected] Test child isolates are terminated when root is shutdown (flutter-team-archive/engine#13048)
2019-10-10 [email protected] Allow embedders to specify arbitrary data to the isolate on launch. (flutter-team-archive/engine#13047)


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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

3 participants