4
\$\begingroup\$

Summary

I have a normal class which has a normal property but that property has access to an async backing field.

I'm not sure if this is the best/acceptable way to access the asyc method for this backing field.

Details

I've got a C# library that helps testing the .NET HttpClient class.

This class has a property public HttpContent HttpContent which is access two private backing fields which some code uses later on.

One of these private backing fields is populated via an async method. This, being a property, isn't async.

When the value of this property is set, I extract the actual string content from it by calling the method _httpContent.ReadAsStringAsync() and also remember a reference to this instance via a class-global variable.

The actual code in question is this

private string _httpContentSerialized;

....

_httpContentSerialized = _httpContent == null
    ? "*"
    : Task.Run(_httpContent.ReadAsStringAsync).Result;

As such, I'm not sure if the code I have (which seems to work, mind you) is the best way to do this and I'm hoping someone can review this (as opposed to asking on SO for a 'how to do this' answer).

Extra Info to consider:

Being a library I have no idea how/where my library will be consumed/used:

  • A console app?
  • An ASP.NET app?

With this in mind, I'm not sure if the code I have written is acceptable and won't cause deadlocks for a .NET application.

Other suggestions have been to use _httpContent.ReadAsStringAsync().ConfigureAwait(false).GetAwaiter().GetResult();


Here's some more of the code:

public class HttpMessageOption
{
    private HttpContent _httpContent;
    private string _httpContentSerialized;

    public HttpContent HttpContent
    {
        get { return _httpContent; }
        set
        {
            _httpContent = value;
            _httpContentSerialized = _httpContent == null
                ? "*"
                : Task.Run(_httpContent.ReadAsStringAsync).Result;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Yeah... you're gonna end up deadlocked. General rule of thumb, don't call the Result property of an awaitable. It's a bad time. Go read everything you can about async by Stephen Cleary. msdn.microsoft.com/en-us/magazine/dn818493.aspx \$\endgroup\$ – RubberDuck Apr 1 '17 at 14:38
5
\$\begingroup\$

You have just literally lost the async part of your ReadAsStringAsync method. By calling the Result property of your Task object, you will block your calling thread since it is going to wait for the async thread to return with the result of the operation. The Result property is just like the Wait method.

You should use await instead of Result or Wait, since:

  1. Result and Wait will encapsulate your Exception into an AggregateException.
  2. You could easily encounter a deadlock situation. You shouldn't block on async code normally.

However, without Result and Wait you simply cannot make a property safely async. It is not supported in C#, only methods, lambda expressions or anonymous methods can use await. Properties should perform fast and non-blocking operations. If you want to perform a long running activity, don't make it a property (or just call a synchronous method in the setter).

private HttpContent Content;
private string SerializedContent;

public async Task SetHttpContent(HttpContent Content)
{
    this.Content = Content;
    SerializedContent = await Content.ReadAsStringAsync();
}

// ...

await SetHttpContent(new object);
\$\endgroup\$
  • \$\begingroup\$ Sweet - I really like what this suggests: when you SET .. it's more than just some backfield that is occuring .. but other magical stuff occurs underneath ... so be warned/here-be-dragons/etc. Now I have another, new problem: ... some point later on ... i'll need to block/wait when I do this SET because the calling code is sync, not async .. but that's another problem for another hour :( \$\endgroup\$ – Pure.Krome Apr 1 '17 at 23:36
4
\$\begingroup\$

I find it is not obvious to the user what happens when he sets the HttpContent and that he needs to actually wait while setting it.

I prefer to have a method like

public async Task<string> GetSerializedHttpContent()
{
    return _httpContent.ReadAsStringAsync();
}

reather a property that does more then I might actually need. What if I never use the _httpContentSerialized but need the HttpContent for other purposes? You might say that you use it but hay, the example contains only a single property so we can only guess about its real usage and the backing field. Nevertheless, it should not do any work but setting a field.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.