Closed lukebakken closed 7 months ago
One thing you must have in mind is that ConfigureAwait(_)
configures the await
and not the taks,
It needs to be wherever the await
is,
@paulomorgado Yes I realize that, but in this case, with the original implementation of AsyncRpcContinuation<T>
, configuring the await
is not an option, which I find surprising.
ConfigureAwait
is an instance method of the Task
and Task<TResult>
classes and ValueTask
and ValueTask<TResult>
structs.
If k
is not one of them, it doesn't have that method.
Check out the ConfigureAwait FAQ.
(I'm sorry, but I'm not on a device where I can check that).
Thank you @paulomorgado for the explanation.
Since AsyncRpcContinuation<T>
instances are on the hot path, I'm assuming it makes sense to continue using ValueTask
.
OK, I have modified AsyncRpcContinuation
to have a WaitAsync()
method rather than GetAwaiter()
in this commit, which resolves this issue:
@lukebakken There was no issue, it was just that the previous version of the code didn't require ConfigureAwait(false)
because it was already returning a correctly configured awaiter. A good analyzer would have caught that and not highlighted that you should add ConfigureAwait(false)
.
It is similar to Task.Yield
. There, you cannot configure the context capturing.
Thanks for the clarification @danielmarbach. I thought the previous version was correct, but I think having an explicit WaitAsync
may be a bit clearer.
I think that should be left only for non-cancelable APIs. All other should already have a cancellation token and that one should be combined with a timeout cancellation token, when a timeout is required.
cc @paulomorgado @danielmarbach
Discussion about
.ConfigureAwait(false)
that prompted this issue: https://github.com/rabbitmq/rabbitmq-dotnet-client/pull/1426#discussion_r1400909896The current implementation of
AsyncRpcContinuation<T>
is such that the awaitable is configured in the class itself, here:https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/client/impl/AsyncRpcContinuations.cs#L72-L78
This means that
await k
does not use.ConfigureAwait(false)
:https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/client/impl/ChannelBase.cs#L256
The original implementation of
AsyncRpcContinuation<T>.GetAwaiter()
returned aTaskAwaiter<T>
:...however, even with the above code,
await k
does not allow.ConfigureAwait(false)
:So, my questions are:
.ConfigureAwait(false)
?