0

I currently have the following:

var tasks = new List<Task>();

foreach (myObject obj in myObjectList)
{
    tasks.Add(downloadBitmap(obj.profilePath, obj.id));
}

await Task.WhenAll(tasks);

downloadBitmap

HttpWebRequest request = (HttpWebRequest)WebRequest.Create(uri);
RequestState myRequestState = new RequestState();
myRequestState.request = request;

// Start the asynchronous request.
IAsyncResult result = request.BeginGetResponse(new AsyncCallback(RespCallback), Tuple.Create(myRequestState, actorID));

// this line implements the timeout, if there is a timeout, the callback fires and the request becomes aborted
ThreadPool.RegisterWaitForSingleObject(result.AsyncWaitHandle, new WaitOrTimerCallback(TimeoutCallback), request, DefaultTimeout, true);

// The response came in the allowed time. The work processing will happen in the  
// callback function.
allDone.WaitOne();

RespCallBack

        Tuple<RequestState, int> state = (Tuple<RequestState, int>)asynchronousResult.AsyncState;
        RequestState myRequestState = state.Item1;
        int actorID = state.Item2;

        try
        {
            HttpWebRequest myHttpWebRequest = myRequestState.request;
            myRequestState.response = (HttpWebResponse)myHttpWebRequest.EndGetResponse(asynchronousResult);

            // Read the response into a Stream object.
            Stream responseStream = myRequestState.response.GetResponseStream();
            myRequestState.streamResponse = responseStream;
            Bitmap bitmap = new Bitmap(responseStream);

            // Do some work here

        }
        catch (WebException e)
        {
            Console.WriteLine("\nRespCallback Exception raised!");
            Console.WriteLine("\nMessage:{0}", e.Message);
            Console.WriteLine("\nStatus:{0}", e.Status);
        }
        finally
        {
            // Release the HttpWebResponse resource.
            myRequestState.response.Close();
        }
        allDone.Set();

I got most of this from the MSDN website. I am also receiving the warning:

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

for the DownloadBitmap function.

I understand that I am not using await in this function but the reason I thought it wasn't necessary anywhere was because BeginGetResponse is already asynchronous?

Not sure if my understanding of this is entirely correct...

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • 1
    In this method you fire up an asynchronous call with `request.BeginGetResponse`. But your current thread is blocked when calling `allDone.WaitOne();` and it's only awake when `another thread` is picked up to process the asynch result and call `allDone.Set();`. It's not the correct way to implement asynch. You could take a look at: http://stackoverflow.com/questions/10565090/getting-the-response-of-a-asynchronous-httpwebrequest – Khanh TO Jan 24 '15 at 04:56
  • Your use of the APM pattern together with WaitHandle makes no sense. Just use `await WebRequest.GetResponseAsync`. You're using await already. Why all this complicated APM stuff? – usr Jan 24 '15 at 10:59
  • Also, this is not a good question for Stack Overflow. It is a wall of code that needs review. – usr Jan 24 '15 at 11:01

3 Answers3

1

BeginGetResponse is asynchronous however it uses an older paradigm of asynchronous programming in .NET called the Asynchronous Programming Model or (APM). Async-await uses a newer way of doing asynchronous programming which is based on the Task Based Asynchronous Pattern.

You can read more about asynchronus programming patterns here.

There is a way to convert from older API supporting APM to newer ones via the TaskFactory.FromAsync method to convert your method to an async one.

I think you'd need something like this:

myRequestState.response = 
       await TaskFactory.FromAsync(
                         request.BeginGetResponse, 
                         request.EndGetResponse,
                         Tuple.Create(myRequestState, actorID));
NeddySpaghetti
  • 13,187
  • 5
  • 32
  • 61
0

If you are using .NET 4.5 or greater, the preferred method to use on a HttpWebRequest object would be GetResponseAsync() see here

So your web call would look something like this:

//note the use of the async/await
public async Task<Bitmap> DownloadBitmap(/* whatever your params were */)
{
    HttpWebRequest request = (HttpWebRequest)WebRequest.Create(uri);
    //add the rest of the data/params or anything else you need then...
    // await the asynchronous request.

    HttpWebResponse result = (HttpWebResponse)(await request.GetResponseAsync());

    Stream responseStream = myWebResponse.GetResponseStream();
    return new Bitmap(responseStream);
}

and call it like this:

var bitmaps = new List<Bitmap>();

foreach (myObject obj in myObjectList)
{
    bitmaps.Add(await DownloadBitmap(obj.profilePath, obj.id));
}

if you don't want to have to await you can do this (or for .NET 4.0+):

//note the use of the the slightly older ContinueWith
public Task<Bitmap> DownloadBitmap(/* whatever your params were */)
{
    HttpWebRequest request = (HttpWebRequest)WebRequest.Create(uri);
    //add the rest of the data/params or anything else you need then...
    // await the asynchronous request.

    return request.GetResponseAsync().ContinueWith((antecedent) => {
        HttpWebResponse myWebResponse = (HttpWebResponse)antencedent.Result;
        Stream responseStream = myWebResponse.GetResponseStream();
        return new Bitmap(responseStream);
    });
}

and call it like this:

var tasks = new List<Task>();

foreach (myObject obj in myObjectList)
{
    tasks.Add(DownloadBitmap(obj.profilePath, obj.id));
}

await tasks.WhenAll();

But that isn't really what you are asking, you are asking about a method being asynchronous or not, using the APM(Asynchronous Programming Model) of IAsyncCallbacks and IAsyncResults is problematic because it requires you to manually configure waits and state and the like source

The TPL(Task Parallel Library) introduces the TAP pattern(Task-based Asynchronous Pattern) cleans up and handles a lot of the heavy lifting for you, by wrapping the waiting and state management with Lambda Expressions and the Task object (my second example).

This can be taken a step further in .NET 4.5 by allowing you to directly await the Task object. rather than needing to call Task.Result(which can be troublesome because exceptions that happen in your Task object are wrapped in an AggregateException object, which you have to unwrap/flatten to see what really happened).

By using async/await in TAP, you are again taking advantage of features built into the language that automatically wait on the task, and return the result, or in the case of an exception, it automatically unwraps the exception and throws it to you like a normal synchronous method.(There is more to it than that, but as far as top-level explanations go, it's that simple - see here for more details.)

It also most importantly takes the clutter out of your methods and allows you to see what is going on in a method more clearly - plus makes it more maintainable.

Finally, to actually answer the question - yes the act of using the BeginGetResponse is asynchronous, however, it is not a call that you can await on because it uses callbacks, rather than returning the object to await.

So while you didn't include it in your question, I'm assuming you have the method signature for DownloadBitmap like so: public async Task DownloadBitmap(/* args */) however, you aren't awaiting anything in that method, because the APM pattern does not directly expose a handle to wait on.

If you want to await on tasks(the preferred method for .NET these days), consider using a different asynchronous pattern.

From MSDN:

Asynchronous Programming Model (APM) pattern (also called the IAsyncResult pattern), where asynchronous operations require Begin and End methods (for example, BeginWrite and EndWrite for asynchronous write operations). This pattern is no longer recommended for new development. For more information, see Asynchronous Programming Model (APM).

Event-based Asynchronous Pattern (EAP), which requires a method that has the Async suffix, and also requires one or more events, event handler delegate types, and EventArg-derived types. EAP was introduced in the .NET Framework 2.0. It is no longer recommended for new development. For more information, see Event-based Asynchronous Pattern (EAP).

Task-based Asynchronous Pattern (TAP), which uses a single method to represent the initiation and completion of an asynchronous operation. TAP was introduced in the .NET Framework 4 and is the recommended approach to asynchronous programming in the .NET Framework. The async and await keywords in C# and the Async and Await operators in Visual Basic Language add language support for TAP. For more information, see Task-based Asynchronous Pattern (TAP).

Community
  • 1
  • 1
tophallen
  • 1,033
  • 7
  • 12
0

I understand that I am not using await in this function but the reason I thought it wasn't necessary anywhere was because BeginGetResponse is already asynchronous?

You're right, you dont need to use the async keyword because you're following a different pattern which is also asynchronous. The one with BeginXXX and EndXXX is called the APM (Asynchronous Programming Model). async-await goes hand in hand with a newer pattern, called TAP (Task-based Asynchronous Pattern).

If you decide to go with the TAP, there is a far easier way to achieve what you're trying to do.

There is no need for any synchronization primitives in this case at all. Use existing TAP apis which exist in HttpClient instead:

public async Task<Bitmap> DownloadBitmapAsync(string uri)
{
    var httpClient = new HttpClient();
    var response = await httpClient.GetAsync(uri);
    using (var responseStream = await response.Content.ReadAsStreamAsync())
    {
        return new Bitmap(responseStream);
    }
}

And then execute them all concurrently:

var bitmapDownloadTasks = myObjectList.Select(obj =>
                                              DownloadBitmapAsync(obj.profilePath));
await Task.WhenAll(bitmapDownloadTasks);

Note i dropped one of the parameters as i couldn't see the method signature for your downloadBitmap method.

This way, you're taking advantage of existing TAP apis instead of creating them yourself with a wrapper over the APM pattern. This can save you several lines of code and can reduce the verbosity your code.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321