Skip to content
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

Allow obtaining serializable tokens that represent JoinableTasks #1162

Merged
merged 1 commit into from
Mar 1, 2023

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Feb 28, 2023

To enable RPC calls (e.g. JSON-RPC -- not COM calls), we add two new APIs:

  1. string? JoinableTaskContext.Capture() - returns a token that represents the JoinableTask that is currently running, and its tokenized parents.
  2. JoinableTask JoinableTaskFactory.RunAsync(Func<Task>, string?, JoinableTaskCreationOptions) - applies a token previously obtained from any Capture() call (even from another process) so that any JoinableTask context that matches this JoinableTaskContext object will be applied as a parent to the new JoinableTask.

Together, this allows RPC infrastructure to utilize JoinableTaskFactory to mitigate deadlocks due to requirements on the same UI thread that would otherwise be undetectable as dependencies due to how RPC runs its own queues, and may weave in and out of the process.

Prior to merging:

Sorry, something went wrong.

@AArnott AArnott added this to the v17.6 milestone Feb 28, 2023
@AArnott AArnott requested a review from lifengl February 28, 2023 19:27
Copy link
Member

@lifengl lifengl left a comment

Choose a reason for hiding this comment

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

those changes look good to me.

@lifengl
Copy link
Member

lifengl commented Mar 1, 2023

One potential thing to consider is that we don't need inject the JTF context into the chain, if the current JTF context has no UI thread. I am not sure this is doable in the JTF side, or you want to leave it to StreamJsonRpc (i guess it is tricky to know that state in StreamJsonRpc), that will allow us to prevent this cost on services which has JTF just to run code in the compatible way, (and I guess most services do not have UI thread), so it could limit the context string to include a single or very few contextId in the real world even the call chain passes through several layers?

@AArnott
Copy link
Member Author

AArnott commented Mar 1, 2023

Yes, I'll look into making Capture() return null when the JoinableTaskContext is in 'no main thread' mode.

@AArnott AArnott enabled auto-merge March 1, 2023 04:20
@AArnott AArnott merged commit 91b5a42 into microsoft:main Mar 1, 2023
@AArnott AArnott deleted the JTCserialize branch March 1, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants