-
Notifications
You must be signed in to change notification settings - Fork 1k
Add async callback support to TrustList implementation #3483
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
Add async callback support to TrustList implementation #3483
Conversation
|
|
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
|
@copilot instead of adding completely new async versions update the internals of the sync version to be async and make a sync wrapper using the .AsTask().GetAwaiter().GetResult() pattern. |
Based on feedback, changed approach: - Make internal implementation async (OpenCoreAsync, etc.) - Sync methods now wrap async using .AsTask().GetAwaiter().GetResult() - Both OnCall and OnCallAsync handlers registered - Eliminates .GetAwaiter().GetResult() in async paths Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Refactored the implementation as requested. The async methods are now the core implementation, and sync methods wrap them using |
Prevents potential deadlocks by adding ConfigureAwait(false) before GetAwaiter().GetResult() in all sync wrapper methods. Also made the pattern consistent across all methods. Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Updated UpdateStoreCertificatesAsync and UpdateStoreCrlsAsync to properly propagate CancellationToken through all async operations. Ensures consistent async patterns throughout the implementation. Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Proposed changes
The TrustList class only supported synchronous
OnCallcallbacks, forcing async operations to block with.GetAwaiter().GetResult(). This adds native async support viaOnCallAsynchandlers while maintaining backward compatibility.Following feedback from @romanett, the implementation uses async as the core with sync wrappers, rather than separate sync and async implementations.
Key changes:
.AsTask().ConfigureAwait(false).GetAwaiter().GetResult()on async core.GetAwaiter().GetResult()patterns in async code paths with properawaitandConfigureAwait(false)CancellationTokenpropagation throughout all async operationsImplementation pattern:
Types of changes
Checklist
Further comments
The implementation follows an async-core pattern where:
await.AsTask().ConfigureAwait(false).GetAwaiter().GetResult()to prevent deadlocksThis approach provides clean async code paths while maintaining full backward compatibility for existing code using sync callbacks.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.