Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[blazor][wasm] Dispatch rendering to main thread #48991
[blazor][wasm] Dispatch rendering to main thread #48991
Changes from all commits
51c7008
6eb9ee8
db1c1c5
4845353
401bf2c
e7c87d5
fe393bf
ffee1e7
068091f
5e56dff
46fd9b7
ae88c7d
2b79ac1
82d97d9
a44a60f
1c7c442
e4e38e0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could you explain the
|| _context == null
part of this check?I would have thought that if
SynchronizationContext.Current == null
, then we'd only consider ourselves "having access" if the original_context
was also null. So it seems like the logic would be more correct if the|| _context == null
part of this check was removed.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.
This looks like a mistake, unless I'm confused.
Shouldn't this return the task returned by
_context.InvokeAsync
, instead ofTask.CompletedTask
?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.
It will invoke the
void SynchronizationContextExtension.InvokeAsync
and so there is no Task. Also it's blocking synchronous call.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.
More details about the expected behavior in https://github.com/dotnet/aspnetcore/pull/48991/files#r1263731418
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.
Doesn't this block? My understanding is that this runs the callback inline, isn't it?
I'm trying to make sense of how this works when you are in a background thread and try to dispatch back to the "main" thread.
My understanding is that the contract for send must block the thread.
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.
Yes, this will block the sender thread. If this is the main thread, it would just invoke it.
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.
The logic around async and exception handling looks strange to me. At least, it will behave very differently from how
RendererSynchronizationContext
does.Dispatcher.InvokeAsync(Action)
WebAssemblyDispatcher
, it blocks the caller until the action is fully complete. If the action throws, then it re-throws instead of returning any task.RendererSynchronizationContextDispatcher
, it doesn't block the caller, and returns aTask
that completes when the action is complete. If the action throws, theTask
is faulted.Dispatcher.InvokeAsync(Func<Task>)
WebAssemblyDispatcher
, it blocks the caller while the dispatch is in process (up until the recipient yields), and then returns theTask
returned by the recipient. If the recipient throws synchronously, then it re-throws instead of returning any task.RendererSynchronizationContextDispatcher
, it does not block the caller, and returns a aTask
that completes when the dispatch and recipient's Task is complete. If the recipent throws synchronously, the returnedTask
is faulted.Are these differences in behavior intentional? My guess is we should behave the same as
RendererSynchronizationContextDispatcher
/RendererSynchronizationContext
.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.
Similarly please see how
RendererSynchronizationContext
deals with unhandled exceptions by callingOnUnhandledException
on theDispatcher
.