Conversation
|
|
||
| const auto tasks = co_await _settings.GlobalSettings().ActionMap().FilterToSnippets(winrt::hstring{}, winrt::hstring{}); // IVector<Model::Command> | ||
| co_await wil::resume_foreground(Dispatcher()); | ||
| co_await wil::resume_foreground(dispatcher); |
There was a problem hiding this comment.
This type of bug in particular is really annoying to spot. :(
There was a problem hiding this comment.
SPECIFICALLY using dispatcher like this without storing it in a strong local? I do not see what the problem is - Dispatcher() is evaluated before the co_await operator
There was a problem hiding this comment.
Because there's another co_await right above this one. this may already be gone. 🫠
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
fwiw I think using strong references in many of these places makes the code easier to read and reason about, at the potential cost of the coroutine keeping the [thing] alive until it's done rather than giving up on it
|
|
||
| safe_void_coroutine TerminalPage::_doHandleSuggestions(SuggestionsArgs realArgs) | ||
| { | ||
| const auto weak = get_weak(); |
There was a problem hiding this comment.
Is an alternative to simply get_strong before switching to the background? For things like this, I don't see the value in vending a weak reference and then immediately resolving it when we can just retain before and release after...
There was a problem hiding this comment.
I didn't do this for fear that this results in issues like XamlRoot being null, after the class got Close()d on the main thread and we continue using it through a coroutine.
microsoft-github-policy-service
bot
added
Needs-Author-Feedback
2 participants
After the coroutines resume,
thismay have already been destroyed.So, this PR adds proper use of
get_weak/strong.Closes #19534
Closes #19745