Isolate platform channels for desktop #35893
Conversation
7a5fbc6 to
f957658
Compare
2981ed5 to
1983ea1
Compare
d19ab9e to
41455fd
Compare
a89fa2e to
0e8de60
Compare
| #if _WIN32 | ||
| // On Windows capturing a TaskRunner with a TaskRunner will cause an | ||
| // uncaught exception in process shutdown because of the deletion order of | ||
| // global variables. See also | ||
| // https://github.com/flutter/flutter/issues/111575. | ||
| // This won't be an issue until Windows supports background platform | ||
| // channels. Then this can potentially be addressed by capturing a | ||
| // weak_ptr to an object that retains the ui TaskRunner, instead of the | ||
| // TaskRunner directly. | ||
| FML_DCHECK(false); | ||
| #endif |
There was a problem hiding this comment.
Typically I wouldn't do this but this situation is pretty difficult to debug and I lost a couple days on it so I don't want to lose it. I have another hypothetical refactor that would make it cleaner until we implement background platform channels but I'm not sure if I'll get to finish it (#36157).
shell/common/shell.cc
Outdated
| // weak_ptr to an object that retains the ui TaskRunner, instead of the | ||
| // TaskRunner directly. |
There was a problem hiding this comment.
(Or link to an existing one for supporting background channels on Windows, with context added there)
| FML_DLOG(WARNING) | ||
| << "Dropping message on channel " << message->channel(); |
There was a problem hiding this comment.
This seems like something that falls through the channel buffer logic we have. I'm not quite sure I understand the inter-relationship there. Can you explain it?
There was a problem hiding this comment.
I'm not suggesting we fully address this in this patch, but maybe we need a bug here, or if not some documentaiton/explanation of why this doesn't apply.
This will end up causing confusing/difficult to debug issues for developers who try to use it and find their messages get dropped (and don't get this log because they're using an opt build).
There was a problem hiding this comment.
The channel buffers are from Host->Flutter direction. This relates to the Flutter->Host direction. The reasons that this else would be executed is that we send a message on the ui thread to an engine that is getting deleted before the message is received on the platform thread. Dropping them is what we've always done, but we've done it silently in the past. I don't think we should drop messages silently, so I have a debug print.
There was a problem hiding this comment.
Ahh, ok. Consider updating this message to say something like "Message received on channel " << message->channel() << ", but engine is deleted. Dropping message." or something.
There was a problem hiding this comment.
Done. I tried to be a bit more succinct: "Deleted engine dropping message on channel " << message->channel()
|
auto label is removed for flutter/engine, pr: 35893, due to - The status or check suite Mac iOS Engine has failed. Please fix the issues identified (or deflake) before re-applying this label. |
eb7a607 to
fb8888d
Compare
|
auto label is removed for flutter/engine, pr: 35893, due to - The status or check suite Linux Fuchsia FEMU has failed. Please fix the issues identified (or deflake) before re-applying this label. |
fb8888d to
7ecde3b
Compare
framework pr: flutter/flutter#110882
issue: flutter/flutter#13937
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.