-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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.
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
Remove the window thread from the list of threads before nulling the AppHost #15231
Remove the window thread from the list of threads before nulling the AppHost #15231
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me. @lhecker?
I'm afraid of these "maybe it fixes it????" changes with the lack of having repros, but I get it! |
|
||
void WindowThread::RundownForExit() | ||
{ | ||
_host = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, (re)move the _host = nullptr
?
Unfortunately, this PR didn't seem to have fixed the issue. While mucking around with 2 tabs, changing order without letting go of them, actually changing their order, dragging into a new window, dragging back into the old window, but not actually, then actually, etc., I had an A/V in the exact same spot. The top most part of the stack trace shows how it's caused by an async action (I think?) inside XAML trying to access an UI element that doesn't exist anymore (I think?). For a double whammy of despair, I'd like to direct your attention to how it uses an IPC system for in-proc calls, because the objects are agile. Might as well use HTTP via localhost. Here's the full stack trace:
|
Exact same spot? I dunno about that - the stack you've got there is quite a bit different than the one in MSFT:43995981. I'm fine calling it an issue, but I'll track it separately. |
See #14957 (comment).
I think there's a race here that lets the WindowEmperor muck around with the window after it's done, but before we remove it from our list of threads.
This should remove the thread from the list, then null out the AppHost, then flush the XAML queue, preventing the A/V.
Closes MSFT:43995981