Object disposal and garbage collection prior to event triggering

Question

A piece of code was brought up by someone I was talking to:

private void DownloadInformation(string id)
{
    using (WebClient wc = new WebClient())
    {
        wc.DownloadStringCompleted += 
            new DownloadStringCompletedEventHandler(DownloadStringCompleted);
        wc.DownloadStringAsync(new Uri("http://www.fake.com/" + id));
    }
}

The above is a simplified version of this:

(I have the author's permission to post the image.)

What bothers me about that code is that an event handler is attached, DownloadStringAsync() is called and then the using block ends, which calls Dispose() on WebClient. Is there anything that will prevent WebClient from being disposed off by using and even garbage collected prior to DownloadStringAsync() completing and DownloadStringCompleted event triggering?

There's a newer method, DownloadStringTaskAsync(), which I would think to use in conjunction with await:

private async Task DownloadInformation(string id)
{
    using (WebClient wc = new WebClient())
    {
        wc.DownloadStringCompleted += DownloadStringCompleted;
        await wc.DownloadStringTaskAsync(new Uri("http://www.fake.com/" + id));
    }
}

However, even then... I would basically be betting that event triggers and handler gets called before the WebClient gets disposed off.

Am I misunderstanding the life cycle of WebClient in this scenario or is this a terrible code design?

You don't need the event with `DownloadStringTaskAsync`. The task should contain the final results. In other words: `var content = await wc.DownloadStringTaskAsync(...);`. — Lasse V. Karlsen, Sep 21 '16 at 06:49
@LasseV.Karlsen Good point -- that would definitely take care of that issue. With that said, I'm definitely uneasy about the first code snippet. — B.K., Sep 21 '16 at 06:50
The first example is horribly broken. Don't use it. Keep the client around until the event completes. — Lasse V. Karlsen, Sep 21 '16 at 06:52
Note that code in originally linked posts was correct - no `using`... — Alexei Levenkov, Sep 21 '16 at 06:55
@AlexeiLevenkov Yes, I linked the wrong one. Can't find that link now (go figure), but I posted the author's image of his code (with his permission). — B.K., Sep 21 '16 at 06:55
Second sample (`await`) is perfectly fine since dispose is called after completion (mixing `async` and events is somewhat strange, but probably ok). — Alexei Levenkov, Sep 21 '16 at 07:01
First one is discussed in http://stackoverflow.com/questions/979791/net-do-i-need-to-keep-a-reference-to-webclient-while-downloading-asynchronousl (and rene's answer) and is awkward, but fine. — Alexei Levenkov, Sep 21 '16 at 07:07
@AlexeiLevenkov Bingo! That is exactly what I was looking for in terms of information, and it links to the other post which goes further into it: http://stackoverflow.com/questions/421547/does-the-garbage-collector-destroy-temporarily-unreferenced-objects-during-async Thank you! — B.K., Sep 21 '16 at 07:12

score 4 · Accepted Answer · edited May 23 '17 at 12:00

4

The WebClient doesn't implement IDisposable, its base class Component does.

The Component class Disposes any eventhandlers that are registered with its Events property but the WebClient doesn't use that property.

Calling Dispose on the WebClient doesn't have an effect on any state managed by the webclient.

The actual handling of internal resources is done in the private method DownloadBits and an internal class DownloadBitsState.

So the code you show is up to now free of any effects due to releasing resources too early. That is however caused by an implementation detail. Those might change in the future.

Due to the framework keeping track of pending callbacks you don't have to worry about premature garbage collection as well, as is explained in this answer, kindly provided by Alexei Levenkov.

edited May 23 '17 at 12:00

Community

1
1

answered Sep 21 '16 at 06:57

rene

41,474
78
114
152

Aside from not implementing its own `Dispose()`, isn't there a possibility that `WebClient` would still get garbage collected prior to event triggering since I don't see anything keeping it alive? Or does `DownloadStringAsync()` which was called on `WebClient` keeps its instance alive until completion? That method returns and nothing keeps reference to the `WebClient` instance. – B.K. Sep 21 '16 at 07:07
Looks like @AlexeiLevenkov found information (in the comments) that confirms that you're correct! – B.K. Sep 21 '16 at 07:16

Lasse V. Karlsen · Answer 2 · 2016-09-21T06:52:53.883

3

The event is only used with DownloadStringAsync, not with DownloadStringTaskAsync.

With the latter, the task is Task<string> and when the task completes it contains the response from the download.

As such the second example can be rewritten as this:

private async Task DownloadInformation(string id)
{
    using (WebClient wc = new WebClient())
    {
        string response = await wc.DownloadStringTaskAsync(new Uri("http://www.fake.com/" + id));
        // TODO: Process response
    }
}

You're entirely right in that the first example is horribly broken. In most cases I would expect the client object to be disposed before the async task completes, and it may even be garbage collected as you're mentioning. The second example, after losing the event, has none of those problems as it will correctly wait for the download to complete before disposing the client object.

edited Sep 21 '16 at 06:52

answered Sep 21 '16 at 06:50

Lasse V. Karlsen

380,855
102
628
825

I would have expected the async keyword too – Sir Rufo Sep 21 '16 at 06:52
2

Second part of this answer is wrong ("broken" part - it is really horrifyingly looking as it look broken). More discussion http://stackoverflow.com/questions/979791/net-do-i-need-to-keep-a-reference-to-webclient-while-downloading-asynchronousl – Alexei Levenkov Sep 21 '16 at 07:09
No, the fact that you dispose an object before you are done with it *regardless of the type of object* is what is horribly broken here. The fact that WebClient *may* work is besides the point. I maintain my stance on that code being horribly broken, even if it *may* work in this specific case. – Lasse V. Karlsen Sep 21 '16 at 08:06

Object disposal and garbage collection prior to event triggering

2 Answers2

Linked