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

Flutter broken because of [ VM / dart:isolate ] Added ability to set names for spawned isolates. #36232

Closed
dcharkes opened this issue Mar 15, 2019 · 5 comments
Closed

Flutter broken because of [ VM / dart:isolate ] Added ability to set names for spawned isolates. #36232

dcharkes opened this issue Mar 15, 2019 · 5 comments
Assignees
@bkonyi
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@dcharkes
Copy link
Contributor

5952526 causes Flutter test observatory and protocol to deadlock (log). Note that this only showed up after the fixing the patch files.

Bisection showing it is this commit:

dacoharkes@dacoharkes-l:~/flt/engine/src/third_party/dart$ git reset --hard 0ee14130955e67b48d6ce88cc0eb9bf22eddb174
HEAD is now at 0ee14130955 [dartdevc] Initial support for control flow collections
dacoharkes@dacoharkes-l:~/flt/engine/src/third_party/dart$ cd ../..
dacoharkes@dacoharkes-l:~/flt/engine/src$ ninja -C out/host_debug
ninja: Entering directory `out/host_debug'
[1158/1158] STAMP obj/default.stamp
dacoharkes@dacoharkes-l:~/flt/engine/src$ third_party/dart/tools/sdks/dart-sdk/bin/dart $PWD/flutter/shell/testing/observatory/test.dart $PWD/out/host_debug/flutter_tester $PWD/flutter/shell/testing/observatory/empty_main.dart
Launching /usr/local/google/home/dacoharkes/flt/engine/src/out/host_debug/flutter_tester [--observatory-port=0, --non-interactive, --run-forever, /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/empty_main.dart]
Observatory listening on http://127.0.0.1:45531/
Executing test 1/3
Executing test 2/3
-> 1 (getVM)
<- 1
-> 2 (BART_SIMPSON)
<- 2
Executing test 3/3
Launching /usr/local/google/home/dacoharkes/flt/engine/src/out/host_debug/flutter_tester [--start-paused, --observatory-port=0, --non-interactive, --run-forever, /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/empty_main.dart]
Observatory listening on http://127.0.0.1:44091/
dacoharkes@dacoharkes-l:~/flt/engine/src$ cd third_party/dart/
dacoharkes@dacoharkes-l:~/flt/engine/src/third_party/dart$ git reset --hard 59525264e82f46a1146ffea79748043ca20fe905
HEAD is now at 59525264e82 [ VM / dart:isolate ] Added ability to set names for spawned isolates.
dacoharkes@dacoharkes-l:~/flt/engine/src/third_party/dart$ cd ../..
dacoharkes@dacoharkes-l:~/flt/engine/src$ ninja -C out/host_debug
ninja: Entering directory `out/host_debug'
[1158/1158] STAMP obj/default.stamp
dacoharkes@dacoharkes-l:~/flt/engine/src$ third_party/dart/tools/sdks/dart-sdk/bin/dart $PWD/flutter/shell/testing/observatory/test.dart $PWD/out/host_debug/flutter_tester $PWD/flutter/shell/testing/observatory/empty_main.dart
Launching /usr/local/google/home/dacoharkes/flt/engine/src/out/host_debug/flutter_tester [--observatory-port=0, --non-interactive, --run-forever, /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/empty_main.dart]
^C

Deadlock state:

$ gdb --args third_party/dart/tools/sdks/dart-sdk/bin/dart $PWD/flutter/shell/testing/observatory/test.dart $PWD/out/host_debug/flutter_tester $PWD/flutter/shell/testing/observatory/empty_main.dart
GNU gdb (GDB) 8.2
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from third_party/dart/tools/sdks/dart-sdk/bin/dart...(no debugging symbols found)...done.
(gdb) r
Starting program: /usr/local/google/home/dacoharkes/flt/engine/src/third_party/dart/tools/sdks/dart-sdk/bin/dart /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/test.dart /usr/local/google/home/dacoharkes/flt/engine/src/out/host_debug/flutter_tester /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/empty_main.dart
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Launching /usr/local/google/home/dacoharkes/flt/engine/src/out/host_debug/flutter_tester [--observatory-port=0, --non-interactive, --run-forever, /usr/local/google/home/dacoharkes/flt/engine/src/flutter/shell/testing/observatory/empty_main.dart]
[Detaching after fork from child process 156257]
^C
Thread 1 "dart" received signal SIGINT, Interrupt.
0x00007ffff79c515f in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0
(gdb) t a a bt

Thread 9 (Thread 0x7ffff03bf700 (LWP 156258)):
#0  0x00007ffff79c8a9e in wait () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x000055555643cd19 in ?? ()
#2  0x000055555645540e in ?? ()
#3  0x00007ffff79bf494 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#4  0x00007ffff6c58a8f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Thread 5 (Thread 0x7ffff24bf700 (LWP 156252)):
#0  0x00007ffff79c515f in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x000055555676d748 in dart::Monitor::WaitMicros(long) ()
#2  0x0000555556946b2a in dart::BackgroundCompiler::Run() ()
#3  0x0000555556811e3d in dart::ThreadPool::Worker::Loop() ()
#4  0x0000555556811cd4 in dart::ThreadPool::Worker::Main(unsigned long) ()
#5  0x000055555676d019 in ?? ()
#6  0x00007ffff79bf494 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#7  0x00007ffff6c58a8f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Thread 2 (Thread 0x7ffff7fcb700 (LWP 156249)):
#0  0x00007ffff6c59083 in epoll_wait () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x0000555556435c55 in dart::bin::EventHandlerImplementation::Poll(unsigned long) ()
#2  0x000055555645540e in ?? ()
#3  0x00007ffff79bf494 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#4  0x00007ffff6c58a8f in clone () from /lib/x86_64-linux-gnu/libc.so.6

Thread 1 (Thread 0x7ffff7fcec00 (LWP 156244)):
#0  0x00007ffff79c515f in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x000055555676d748 in dart::Monitor::WaitMicros(long) ()
#2  0x000055555697cbca in Dart_RunLoop ()
#3  0x000055555642db11 in dart::bin::RunMainIsolate(char const*, dart::bin::CommandLineOptions*) ()
#4  0x000055555642e631 in dart::bin::main(int, char**) ()
#5  0x000055555642eee9 in main ()
dart-bot pushed a commit that referenced this issue Mar 15, 2019
@dcharkes
Revert "[ VM / dart:isolate ] Added ability to set names for spawned …
638f13f 
…isolates."

This reverts commit 5952526.

Reason for revert: causes Flutter test observatory and protocol to deadlock.
Issue: #36232

Original change's description:
> [ VM / dart:isolate ] Added ability to set names for spawned isolates.
> 
> Fixes issue #34059
> 
> Change-Id: I315498b02edc184e9e408c93eddb78aa1a5a8a1d
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/90341
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>

TBR=bkonyi@google.com,rmacnak@google.com,asiva@google.com,sigmund@google.com

Change-Id: I5f2115a2ac394a8d4c7c175bc97f2b88b65fcb49
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97107
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
@dcharkes
Copy link
Contributor Author

dcharkes commented Mar 15, 2019 •
edited
Loading

Reverting the commit also made the set_name and isolate_lifecycle tests fail. (As it reverted the name functionality.)

| BOT                              | TEST                           | RESULT       | EXPECTED | 
| -------------------------------- | ------------------------------ | ------------ | -------- | 
| vm-kernel-linux-debug-simdbc64   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm   | service/isolate_lifecycle_test | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm64 | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simdbc64 | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-mac-release-simdbc64   | service/isolate_lifecycle_test | RuntimeError | Pass     |
| vm-kernel-mac-release-simdbc64   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-mac-debug-simdbc64 | service/set_name_rpc_test | RuntimeError | Pass     |

@bkonyi
Copy link
Contributor

bkonyi commented Mar 15, 2019

Reverting the commit also made the set_name and isolate_lifecycle tests fail. (As it reverted the name functionality.)

| BOT                              | TEST                           | RESULT       | EXPECTED | 
| -------------------------------- | ------------------------------ | ------------ | -------- | 
| vm-kernel-linux-debug-simdbc64   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm   | service/isolate_lifecycle_test | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simarm64 | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-linux-release-simdbc64 | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-mac-release-simdbc64   | service/isolate_lifecycle_test | RuntimeError | Pass     |
| vm-kernel-mac-release-simdbc64   | service/set_name_rpc_test      | RuntimeError | Pass     |
| vm-kernel-mac-debug-simdbc64 | service/set_name_rpc_test | RuntimeError | Pass     |

Why are these tests failing now? They shouldn't as changes to those tests were made as part of the reverted commit.

@vsmenon vsmenon added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Mar 17, 2019
@sstrickl
Copy link
Contributor

cross-vm-linux-release-arm64 was missed, so approving that now until we can find the underlying issue (which I'll dig into once I finish initial gardening stuff)

@sstrickl
Copy link
Contributor

For the set_name_rpc_test failures, it's that the isolate.name is out.dill:main() but it expects one of set_name_rpc_test.dart:main() or out.jitsnapshot:main(). Given that, maybe some change landed between the initial commit and the revert that changed the behavior on these platforms to create out.dill instead of an out.jitsnapshot, since that part is coming from the script URI? An easy fix would be to just add another alternative for out.dill:main(), and I may do that once I've verified that there was such a change.

@sstrickl
Copy link
Contributor

Similar story there: the isolate_lifecycle_test looks for 'main' as a final suffix to isolate names (so as to only work on spawned entries), but the actual suffix is 'main()', so it only ends up affecting one of the spawned entries (and attempting to resume the main isolate as well). I'll go ahead and put together a quick CL to fix both of these issues.

@bkonyi bkonyi mentioned this issue Mar 21, 2019
Merged
dart-bot pushed a commit that referenced this issue Mar 22, 2019
@sstrickl
[gardening] Fix up name detection in isolate tests.
81b984d 
This change addresses the test failures seen in
#36232.

Change-Id: I062669c07f4963291450b35a46fb73256505c706
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97220
Auto-Submit: Stevie Strickland <sstrickl@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Stevie Strickland <sstrickl@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bkonyi bkonyi

Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sstrickl @vsmenon @dcharkes @bkonyi