Skip to content

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Mar 8, 2018

Fixes #14636 ... hopefully. My understanding after some debugging is that ResidentRunner attempts to print "Service protocol connection closed." when the device-side VM service closes the connection. Simultaneously, the resident process itself is attempting to shutdown. Hence the race: if the resident process shuts down first, it never gets the notification about the severed connection and makes no attempt to print the aforementioned string. However, if it loses the race it attempts to use Stdio from the AppContext in the current Zone. Unfortunately, the daemon never adds Stdio to the AppContext and so it crashes with NoSuchMethodError.

The fix seems to be to simply add Stdio to the AppContext 🤞

/cc @DanTup @tvolkert

@tvolkert
Copy link
Contributor

tvolkert commented Mar 8, 2018

But when it's not in the AppContext in the current zone, it should find the parent AppContext, which should have it there.

Can you set up the code to force it to lose the race in order to verify this theory?

@yjbanov
Copy link
Contributor Author

yjbanov commented Mar 8, 2018

I thought this was the parent context, but let me test this theory. Also, I'll check if I can reliably reproduce this. I only get this in 20% cases on one of the devicelab machines.

@tvolkert
Copy link
Contributor

tvolkert commented Mar 8, 2018

I thought this was the parent context

If this is being run via flutter daemon, then it goes through the bootstrap runner, which sets up the parent context here:

return await _executableContext.runInZone(() async {

@yjbanov
Copy link
Contributor Author

yjbanov commented Mar 9, 2018

This happens when running flutter run.

@DanTup
Copy link
Contributor

DanTup commented Mar 9, 2018

flutter run --machine, if that makes any difference?

@yjbanov
Copy link
Contributor Author

yjbanov commented Mar 9, 2018

@tvolkert and I just had a offline conversation. The issue seems to be in the AppContext's zone management code, where it incorrectly flips pointers from one zone to another. The long-term fix is to stop doing that and make AppContext immutable. It is a small change in AppContext but will have a huge effect on the rest of flutter_tools package using it. To unblock this specific issue we agreed to accept this fix and implement a long-term fix as a follow-up. @tvolkert is writing up an issue for that as we speak.

Copy link
Contributor

@tvolkert tvolkert left a comment

Choose a reason for hiding this comment

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

LGTM

@tvolkert
Copy link
Contributor

tvolkert commented Mar 9, 2018

Full issue described here: #15352

@yjbanov yjbanov merged commit 2e429bf into flutter:master Mar 9, 2018
DaveShuckerow pushed a commit to DaveShuckerow/flutter that referenced this pull request May 14, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

run_machine_concurrent_hot_reload devicelab test is flaky

4 participants