2

I have a small MVC 5 application that calls a web service, and receives a JSON response. I deserialise the response into my own type and it gets passed on to the view and data is displayed by razor.

The controller handler:

    public async Task<ActionResult> Search(string q)
    {
        var vm = new SearchResultViewModel(await _searchService.GetDataAsync(q));
        return View(vm);
    }

The search service method:

    public async Task<ISearchResult> GetDataAsync(string q)
    {
        var fullRequest = new UriBuilder(RequestUri) {Query = "q=" + q};

        var result = await _client.GetAsync(fullRequest.ToString()).ConfigureAwait(false);

        if (result.IsSuccessStatusCode)
        {
            var jsonResponse = await result.Content.ReadAsStringAsync().ConfigureAwait(false);

            // How should I call this?
            return JsonConvert.DeserializeObject<SearchResult>(jsonResponse);
        }
        return new SearchResult
    }

My question: How should I call JsonConvert.DeserializeObject? It's an inherently CPU bound operation, so is it ok to call synchronously (and block the thread) since I can't return until it's done anyway? If there's a problem with deserialisation, a cancellation token couldn't be used.

If I should call asynchronously, should I use Task.Factory.StartNew() as suggested by intellisense, as a replacement for the deprecated JsonConvert.DeserializeObjectAsync()? This Channel 9 video suggests (at 58mins) that this isn't such a good idea. Perhaps another option, such as Task.Run()? Possibly a bad idea since it might cause SyncContext issues?

Any pointers gratefully received!

juvchan
  • 6,113
  • 2
  • 22
  • 35
Simon B
  • 976
  • 1
  • 11
  • 20
  • 1
    I don't think there is any difference between `Task.Factory.StartNew()` and `Task.Run()` except if you don't want to use the threadpool. I would say your code is fine as is (eg deserialize synchronously) unless the JSON is likely to be large in which case you could do a buffered data read from your `result.Content` as a stream (if possible) and deserialize the JSON in a streaming manner rather than allocating the entire `string` first. – jamespconnor Mar 12 '16 at 23:33
  • 2
    @SimonB: For future reference, do not use `Task.Factory.StartNew`; it has dangerous defaults. `Task.Run` is safer. However, in most cases, neither should be used on ASP.NET. – Stephen Cleary Mar 14 '16 at 15:52
  • @StephenCleary Thanks for the heads up! I'm not sure what the use cases are for ASP.NET, particularly for CPU-bound ops. Perhaps if you have some parallel tasks to run? Obviously high parallelism has its own issues in a web server context. – Simon B Mar 15 '16 at 23:39
  • @SimonB: CPU-bound code should just be run directly on ASP.NET. I do not recommend any parallelism in web apps. – Stephen Cleary Mar 16 '16 at 11:22

1 Answers1

3

Your code is good as is. DeserializeObject will run inside a thread-pool thread since you are using ConfigureAwait(false).

Your overall method (GetDataAsync) would still be asynchronous since it will return to the caller on the first await.

Yacoub Massad
  • 27,509
  • 2
  • 36
  • 62
  • 1
    Side note: question marked with ASP.Net where using `ConfigureAwait(false)` does not do any good (using it is generally cargo cult programming). It also actively harmful if code after the async call needs access to request or current culture (see http://stackoverflow.com/questions/13489065/best-practice-to-call-configureawait-for-all-server-side-code for long discussion) – Alexei Levenkov Mar 12 '16 at 23:35