ConfigureAwait(false) for close stream/consumer/producer, query offset and create producer

[iuribrindeiro]

The changes in #230 were good but not enough to fix deadlocks when trying to close consumers connections.
Adding a ConfigureAwait(false) only to MaybeClose was not enough because when it was being called by RawConsumer.Close like this:

await _client.MaybeClose($"_client-close-subscriber: {_subscriberId}");

A deadlock was still happening.
The ConfigureAwait in .NET is kind of... eeh.. stupid. You basically have to call it for every await you do inside library code because, other wise, you would be getting the application's SynchronizationContext. Since this code is supposed to be used anywhere, we don't actually know how the client SynchronizationContext works, we don't know it's limitations. If it is a single thread SynchronizationContext, for example, we would be causing a deadlock awaiting on the current thread to finish a job that it self started.

See the following example:

SynchronizationContext.SetSynchronizationContext(new MainThreadSynchronizationContext());
await LibraryCode();

async Task LibraryCode()
{
    await DoWork();
}

async Task DoWork()
{
    await Task.Delay(1000).ConfigureAwait(false);
}

On the example above, calling ConfigureAwait(false) on the Task.Delay(1000) method won't be enough. It will, for unblocking code after Task.Delay(1000), but once we are back to the last await(in LibraryCode method), the application SynchronizationContext will once again be used. That way we are now waiting on the current thread to finish work that it self started, making it busy to listen for the completed stated of DoWork. Deadlock.

If we simply add:

  await DoWork().ConfigureAwait(false);

It will now be the app responsability to deal with the task returned by the Task LibraryCode method.
It could either call an await like it is doing in our example above (but if so, would need to call .ConfigureAwait(false) on the LibraryCode as well. Same thing as the last paragraph), or it could do a while(!task.IsCompleted) {} to run code only after the task is completed.

TLDR:
ConfigureAwait(false) everywhere for library code.

Reference: https://devblogs.microsoft.com/dotnet/configureawait-faq/#:~:text=ConfigureAwait(false)%20involves%20a%20task,context%20that%20was%20there%20previously.

[lukebakken]

@iuribrindeiro thanks for doing this. I have added a code analysis warning for CA2007 for the library (not the tests). Some of the warnings I have fixed with the "Roslynator" tool, and the remaining ones need to be fixed manually.

I can get that this weekend, but if you do that work, be sure to keep your branch updated so we don't duplicate it. Thanks!

[iuribrindeiro]

@iuribrindeiro thanks for doing this. I have added a code analysis warning for CA2007 for the library (not the tests). Some of the warnings I have fixed with the "Roslynator" tool, and the remaining ones need to be fixed manually.

I can get that this weekend, but if you do that work, be sure to keep your branch updated so we don't duplicate it. Thanks!

Awesome! I'll do a few and keep updaing this PR

[iuribrindeiro]

@iuribrindeiro thanks for doing this. I have added a code analysis warning for CA2007 for the library (not the tests). Some of the warnings I have fixed with the "Roslynator" tool, and the remaining ones need to be fixed manually.

I can get that this weekend, but if you do that work, be sure to keep your branch updated so we don't duplicate it. Thanks!

@lukebakken I've set the dotnet_diagnostic.CA2007.severity to error and fixed all the missing ConfigureAwait(false) that was causing errors on VS(not sure if that was a Resharper thing or VS by default). Then I moved dotnet_diagnostic.CA2007.severity back to warning. I think that there are no ConfigureAwait(false) missing in RabbitMQ.Stream.Client. Not sure if we need it for the tests though.

[Gsantomaggio]

Thank you @iuribrindeiro