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.

Sign up for GitHub

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

Jump to bottom

Enable shutting down all root isolates in a VM. #8402

Merged
merged 5 commits into from
Apr 3, 2019
Merged

Enable shutting down all root isolates in a VM. #8402

merged 5 commits into from
Apr 3, 2019

Conversation

chinmaygarde
Copy link
Member

No description provided.

cbracken
cbracken reviewed Apr 2, 2019
View reviewed changes
shell/common/isolate_configuration.cc
if (settings.application_kernels) {
return CreateForKernelList(settings.application_kernels());
}

Copy link
Member

Choose a reason for hiding this comment

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

Add a TODO to kill off the following conditional block

Copy link
Member Author

Choose a reason for hiding this comment

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

I have noted that the fields in the settings are deprecated. Once those are removed, this check will necessarily have to go as well.

@chinmaygarde
Copy link
Member Author

Hmm, how did this break every test known to man :(

cbracken
cbracken approved these changes Apr 3, 2019
View reviewed changes
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

lgtm_hanko

runtime/dart_isolate.cc Show resolved Hide resolved
@chinmaygarde chinmaygarde merged commit b59c443 into flutter:master Apr 3, 2019
@chinmaygarde chinmaygarde deleted the vmdata branch April 3, 2019 22:44
chinmaygarde added a commit that referenced this pull request Apr 4, 2019
@chinmaygarde
Revert "Enable shutting down all root isolates in a VM. (#8402)"
0518927 
This reverts commit b59c443.
@chinmaygarde chinmaygarde mentioned this pull request Apr 4, 2019
Merged
@cbracken
Copy link
Member

cbracken commented Apr 4, 2019 •
edited
Loading

Crash on the bot:

(lldb) bt
* thread #2, name = 'Dart ThreadPool Worker', stop reason = EXC_BAD_ACCESS (code=1, address=0x470)
  * frame #0: 0x00007fff7336d11b libsystem_pthread.dylib`pthread_mutex_lock
    frame #1: 0x00007fff70853b29 libc++.1.dylib`std::__1::mutex::lock() + 9
    frame #2: 0x00000001000d4af1 embedder_unittests`std::__1::lock_guard<std::__1::mutex>::lock_guard(this=0x0000700004bf4410, __m=0x0000000000000470) at __mutex_base:104
    frame #3: 0x00000001000d23cb embedder_unittests`std::__1::lock_guard<std::__1::mutex>::lock_guard(this=0x0000700004bf4410, __m=0x0000000000000470) at __mutex_base:104
    frame #4: 0x0000000100224e51 embedder_unittests`blink::DartVM::UnregisterActiveIsolate(this=0x0000000000000000, isolate=std::__1::shared_ptr<blink::DartIsolate>::element_type @ 0x0000000107000620 strong=4 weak=2) at dart_vm.cc:500
    frame #5: 0x00000001001ed3a0 embedder_unittests`blink::DartIsolate::OnShutdownCallback(this=0x0000000107000620) at dart_isolate.cc:763
    frame #6: 0x00000001001ed30b embedder_unittests`blink::DartIsolate::DartIsolateShutdownCallback(embedder_isolate=0x0000000105e00820) at dart_isolate.cc:735
    frame #7: 0x000000010189dce7 embedder_unittests`dart::Dart::RunShutdownCallback() at dart.cc:817
    frame #8: 0x0000000101a5463c embedder_unittests`dart::RunServiceTask::ShutdownIsolate(parameter=4412411392) at service_isolate.cc:388
    frame #9: 0x0000000101915986 embedder_unittests`dart::MessageHandler::TaskCallback(this=0x0000000105e01d80) at message_handler.cc:487
    frame #10: 0x0000000101a77e9d embedder_unittests`dart::ThreadPool::Worker::Loop(this=0x0000000105e03460) at thread_pool.cc:381
    frame #11: 0x0000000101a77d14 embedder_unittests`dart::ThreadPool::Worker::Main(args=4393546848) at thread_pool.cc:436
    frame #12: 0x00000001019c66d1 embedder_unittests`dart::ThreadStart(data_ptr=<unavailable>) at os_thread_macos.cc:110
    frame #13: 0x00007fff7336f305 libsystem_pthread.dylib`_pthread_body + 126
    frame #14: 0x00007fff7337226f libsystem_pthread.dylib`_pthread_start + 70
    frame #15: 0x00007fff7336e415 libsystem_pthread.dylib`thread_start + 13

chinmaygarde added a commit that referenced this pull request Apr 4, 2019
@chinmaygarde
Revert "Enable shutting down all root isolates in a VM. (#8402)" (#8431)
800ea0a 
This reverts commit b59c443.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 4, 2019
@engine-flutter-autoroll
b59c443 Enable shutting down all root isolates in a VM. (flutter/engi…
c25ded2 
…ne#8402)
This was referenced Apr 4, 2019
Closed
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 4, 2019
@engine-flutter-autoroll
b59c443 Enable shutting down all root isolates in a VM. (flutter/engi…
62b33a4 
…ne#8402)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 4, 2019
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 4, 2019
@engine-flutter-autoroll
b59c443 Enable shutting down all root isolates in a VM. (flutter/engi…
46b1a33 
…ne#8402)
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Apr 4, 2019
@engine-flutter-autoroll
Roll engine 309d078..ad9d241 (15 commits) (#30502)
21e34e0 
flutter/engine@309d078...ad9d241

git log 309d078..ad9d241 --no-merges --oneline
ad9d241 Roll src/third_party/skia dde12a25bb5e..f346df396a23 (1 commits) (flutter/engine#8438)
0ed237c Roll src/third_party/skia b6d04556c34d..dde12a25bb5e (4 commits) (flutter/engine#8437)
445ab42 Roll src/third_party/dart 11a3dd7332..600b2831ef (9 commits)
49d0dc6 Roll src/third_party/skia 5c5de2146309..b6d04556c34d (1 commits) (flutter/engine#8434)
4ff18fb Roll src/third_party/dart f6768b6fb3..11a3dd7332 (56 commits)
7b83198 Roll src/third_party/skia 2273c900ea28..5c5de2146309 (5 commits) (flutter/engine#8432)
800ea0a Revert &#34;Enable shutting down all root isolates in a VM. (#8402)&#34; (flutter/engine#8431)
b59c443 Enable shutting down all root isolates in a VM. (flutter/engine#8402)
9acdf18 Make AccessibilityViewEmbedder final (flutter/engine#8429)
804d54d Eliminate unused displayBounds parameter (flutter/engine#8427)
d4c815f Roll buildroot (flutter/engine#8425)
20190d7 Roll src/third_party/skia fbe240669394..2273c900ea28 (13 commits) (flutter/engine#8428)
7e38b0a Revert &#34;Revert &#34;Separate the data required to bootstrap the VM into its own class. (#8397)&#34; (#8406)&#34; (flutter/engine#8414)
d8bb9d7 Roll src/third_party/skia 176da0bb8a10..fbe240669394 (9 commits) (flutter/engine#8424)
64fc583 Remove use of DART_CHECK_VALID. (flutter/engine#8417)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (chinmaygarde@google.com), and stop
the roller if necessary.
RBogie pushed a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@chinmaygarde @RBogie
Enable shutting down all root isolates in a VM. (flutter#8402)
3b59054
RBogie pushed a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@chinmaygarde @RBogie
Revert "Enable shutting down all root isolates in a VM. (flutter#8402)…
28b5ecd 
…" (flutter#8431)

This reverts commit b59c443.
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@RBogie
Revert "Revert "Enable shutting down all root isolates in a VM. (flut…
7b7f7ee 
…ter#8402)" (flutter#8431)"

This reverts commit 28b5ecd.
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@RBogie
Revert "Enable shutting down all root isolates in a VM. (flutter#8402)"
322e63d 
This reverts commit 3b59054.
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@RBogie
Revert "Revert "Enable shutting down all root isolates in a VM. (flut…
82ef84e 
…ter#8402)" (flutter#8431)"

This reverts commit 28b5ecd.
RBogie added a commit to RBogie/flutter-engine that referenced this pull request Apr 8, 2019
@RBogie
Revert "Enable shutting down all root isolates in a VM. (flutter#8402)"
08ac5e4 
This reverts commit 3b59054.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbracken cbracken cbracken approved these changes

@GaryQian GaryQian Awaiting requested review from GaryQian

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
3 participants
@chinmaygarde @cbracken @googlebot