views:

624

answers:

5

I use the following method in a piece of production code:

    private void DownloadData(Uri uri)
    {
      WebClient webClient = new WebClient();
      DownloadDataCompletedEventHandler eh = null;
      eh = delegate(object sender, DownloadDataCompletedEventArgs e)
           {
             webClient.DownloadDataCompleted -= eh;
             ((IDisposable) webClient).Dispose();
             OnDataDownloaded();
           };
      webClient.DownloadDataCompleted += eh;
      webClient.DownloadDataAsync(uri);
    }

I am now worried that a hard to reproduce bug might be caused by the webClient being garbage collected before the DownloadDataCompleted-event is called: after exiting my DownloadData()-method, there are no obvious references to the WebClient-object, so that could plausibly happen.

So my question is: can this realistically happen? I can not reproduce the problem, so there might be some internal things happening that prevents the WebClient from being garbage collected (e.g. the object might register itself with a global object somewhere while waiting for the response.

The code is running on .NET 2.0 if that makes any difference.

+2  A: 

I don't know for sure whether the WebClient can normally be garbage collected or not while an async operation is in progress, because there may be internal references - but the bigger question is: does it matter?

So long as enough of the WebClient stays "alive" to service the request and call your handler, does it matter whether the main WebClient object itself is garbage collected?

The WebClient documentation doesn't mention anything about having to hold onto a reference (unlike the System.Threading.Timer docs, for instance) so I think it's reasonable to assume this is okay.

In this particular case, your delegate has a reference to the WebClient, so as long as the delegate itself is referenced, the WebClient can't be. My educated guess is that some part of the system somewhere needs to hold a callback to know what to do when network traffic arrives, and that callback will eventually (indirectly) lead to your delegate, so you're okay.

Jon Skeet
You are of course correct that the important question is whether the delegate gets garbage collected. And yes, one should think that the IO completion system needs to hold a (indirect) reference to the delegate. But it does not seem completely impossible that this reference might be a WeakReference.
Rasmus Faber
Yup, not impossible. But I would *hope* that would be documented (as it is for Timer).
Jon Skeet
A: 

Flipside of the coin... if you stored a reference to the WebClient somewhere, just to see if it makes a difference... does that make the problem go away altogether? It may be easier to check it this way and be sure than to guess at what seems logical.

jerryjvl
+1  A: 

You can try debugging the application with Debugging Tools for Windows - it allows you to see what exactly is keeping references to a specific object (with the appropriate plug-in). Very useful for such cases.

I do not know the answer to your question, though. One possibility is that for the duration of the operation, WebClient makes itself one of the "root" objects, which are never garbage collected (a .NET application generally has around 5 to 10 such objects, which are the roots of several reference trees used by the application). This is pure speculation, though.

Sander
A: 

When creating an anonymous method with a reference to a scope variable (webClient in your case) and make it's own variable with a reference to that object. So As jon is guessing your delegate will hold a reference to the webClient and before the delegate unregisters it self the webClient can't be garbage collected.

However I'd generally suggest not to use the webClient reference in your delegate method but to cast the sender to an internal variable. using variables externally to anonymous methods can lead to some very strange bugs indeed.

Rune FS
+1  A: 

No, your object won't be GC-ed until the callback completes. According to http://stackoverflow.com/questions/421547/does-the-garbage-collector-destroy-temporarily-unreferenced-objects-during-async, "the async API keeps a reference to your request (within the thread pool where async IO operations are lodged) and so it won't be garbage collected until it completes."

But, your code is also doing stuff it doesn't need to: you don't need to detach the event handler and don't need to call Dispose on the webclient. (Dispose() is actually not implemented by WebClient-- yiou can can see this in the .NET Framework reference source at http://referencesource.microsoft.com/netframework.aspx).

So you don't actually need to refer to the webclient instance in your callback. In other words, the following code will work just as well, and avoid any potential issues (discussed above) of referencing external local variables from inside a delegate.

private void DownloadData(Uri uri)
{
    WebClient webClient = new WebClient();
    DownloadDataCompletedEventHandler eh = null;
    eh = delegate(object sender, DownloadDataCompletedEventArgs e)
    {
        OnDataDownloaded();
    };
    webClient.DownloadDataAsync(uri);
}

Anyway, you probably want to look elsewhere for the source of your bug. One place I'd look is at the results of the HTTP calls-- you may be running out of memory, may be running into server errors, etc. You can look at e.Error to see if the calls are actually working.

Justin Grant