3

i have following code

ThreadPool.QueueUserWorkItem(new WaitCallback(DownloadAsync), apiMethod);
downloadHandle.WaitOne();

Where DownloadAsync is

private void DownloadAsync(object _uri)
        {
            var url = _uri as string;
            WebClient client = new WebClient();
            client.DownloadStringCompleted += new DownloadStringCompletedEventHandler(client_DownloadStringCompleted);
            client.DownloadStringAsync(new Uri(GLOBALS.MAIN_API_URL + url, UriKind.Absolute));
        }

void client_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
        {
            result = e.Result;
            downloadHandle.Set();
        }

So my problem is that downloadHandle.Set() will never called. But i don't understand why? I Create a new thread for DownloadAsync and downloadHandle.WaitOne() shouldn't block him.

That i need is create a Sync method instead of Async.

Thanks!

UPD: Updated source code with Async calling.

Roman Golenok
  • 1,427
  • 9
  • 26

4 Answers4

4

client.DownloadString is synchronous method, so your completed handler will never be called. You need to call asynchronous version: client.DownloadStringAsync()

You can read more about DownloadStringAsync on msdn. It's also wise to put code in try-catch block and handle exceptions if you are relying on the fact, that some code should be called.

Your code could look like this:

private void DownloadAsync(object _uri)
{
    try
    {
        var url = _uri as string;
        WebClient client = new WebClient();
        client.DownloadStringCompleted += new DownloadStringCompletedEventHandler(client_DownloadStringCompleted);
        client.DownloadStringAsync(new Uri(GLOBALS.MAIN_API_URL + url, UriKind.Absolute));
    }
    catch //appropriate exception
    {
       //Handle exception (maybe set downloadHandle or report an error)
    }
}

void client_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
{
    result = e.Result;
    downloadHandle.Set();
}
Marcin Deptuła
  • 11,789
  • 2
  • 33
  • 41
  • ...and of course there's this :-) -- but also note that you don't have to use the thread pool here to call DownloadAsync. – Sasha Goldshtein Feb 20 '12 at 09:52
  • @SashaGoldshtein Of course you are right, I didn't spot this before. +1 – Marcin Deptuła Feb 20 '12 at 09:57
  • It's not working anyway =) client_DownloadStringCompleted will never called during downloadHandle is not set. – Roman Golenok Feb 20 '12 at 10:17
  • @RomanGolenok - Did you called DownloadAsync directly and are you sure, that you have no exception before calling DownloadStringAsync, for example by catching exception or trying to log something after DownloadStringAsync()? – Marcin Deptuła Feb 20 '12 at 10:19
  • I'm sure. client.DownloadStringAsync is called, but event handler not. So, if i replace ManualResetEvent to AutoResetEvent with duration 3 sec, after this 3 secs client_DownloadStringCompleted will be called, but downloadHandle.WaitOne too and result will be empty in this time. – Roman Golenok Feb 20 '12 at 10:23
  • @RomanGolenok - If you are calling DownloadAsync from UI thread, try to ommit `.WaitOne()` and exit from method. Your code right now has no advantage over standard synchronous call. You should call async method and return from calling method, and in completed handler put rest of your logic. If you want to call async method and wait for handler completion, call both async method and handler.WaitOne() on other thread. – Marcin Deptuła Feb 20 '12 at 12:29
2

There might be an exception that prevents the completion callback method from being called. Did you check whether it was called at all?

By the way, you don't actually need to use the thread pool here -- you can call DownloadAsync() on your main thread, because it does not block.

Sasha Goldshtein
  • 3,499
  • 22
  • 35
2

My guess: you're calling downloadHandle.WaitOne() from the UI thread. If you're executing code from an UI event handler (for instance, a click to a button, or a navigation to a new page), or a function called by the event handler, then you're in the UI thread.

Why is this a problem?

Sure enough, DownloadAsync is executed in the background, thanks to the threadpool. However, the WebClient class always executes its callback (that is, your client_DownloadStringCompleted method) using the UI thread... But this very same thread is blocked by your downloadHandle.WaitOne()!

That's why, when you put a timeout on your lock, the client_DownloadStringCompleted method gets magically executed.

How to fix this? Two solutions.

1/ Stop executing downloadHandle.WaitOne() in the main thread. It blocks the user interface and your application becomes unresponsive, which is never a good thing.

2/ Use HttpWebRequest instead of WebClient. HttpWebRequest executes the callback on the same thread that started the download, so you won't have this deadlock issue.

Kevin Gosse
  • 38,392
  • 3
  • 78
  • 94
0

When you call downloadHandle.WaitOne(); from UI you block UI thread, so ThreadPool will never called. Move to background this:

ThreadPool.QueueUserWorkItem(new WaitCallback(DownloadAsync), apiMethod);
downloadHandle.WaitOne();
Ku6opr
  • 8,126
  • 2
  • 24
  • 31
  • What do you mean move to background? This code is processing in ViewModel, so UI thread is not blocked. – Roman Golenok Feb 20 '12 at 10:20
  • If you call `ViewModel` method from `UI` it will be executed on `UI`. But I don't know the context of your call, so I just guess where was an issue. – Ku6opr Feb 20 '12 at 10:41