0

This example below I found while looking answer to another quiestion. Here that guy disposes response in finally block. Is it really necessary? Is it a GC's work in this case?

public static async Task EnsureSuccessStatusCodeAsync(this HttpResponseMessage response)
    {
        try
        {
            if (response.IsSuccessStatusCode)
                return;
            var content = await response.Content.ReadAsStringAsync();
            throw new SimpleHttpResponseException(response.StatusCode, content);
        }
        finally
        {
            response.Content?.Dispose();
        }
    }
AsValeO
  • 2,859
  • 3
  • 27
  • 64
  • 2
    I don't like very much disposing an object passed as a parameter... it is something a little... I don't know... Not beautiful... Not bad bad, but not good good. – xanatos Aug 13 '15 at 14:28
  • 1
    But the code is correct... If possible/not too much difficult, you should always `Dispose()` `IDisposable` objects... It helps the GC... and you don't have any guarantee the GC will ever run. If there is no memory pressure, the GC could not run for a long time. – xanatos Aug 13 '15 at 14:29
  • @xanatos - The GC **never** ever calls `.Dispose()`. You must explicitly do it. – Enigmativity Dec 18 '15 at 02:27

1 Answers1

2

The whole point of using IDisposable is to clean up unmanaged resources that the GC can't clean up on its own. So no, you can't just let the GC clean it up because, by definition, it can't.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • 2
    A correctly implemented `IDisposable` pattern has a finalizer if it manages unmanaged resources, so even if `Dispose()` isn't called, at the next GC run everything fill be fixed. Incorrect `IDisposable` patterns are by definition incorrect, so they could even not free everything :-) So the only use of `Dispose()` is to guarantee a deterministic and timely freeing of the unmanaged resources. – xanatos Aug 13 '15 at 14:34
  • The question though becomes, at least in this case, because the method doesn't *own* the `HttpResponseMessage`, should the method be calling `Dispose` on it (or its contents). If the owner then calls the `Dispose` this will cause an exception. I tried looking for the source, but I would assume disposing the `HttpResponseMessage` calls `Dispose` on its `Content` property, would this not cause hard-to-trace issues later? – Ron Beyer Aug 13 '15 at 14:37
  • 1
    @xanatos finalizers *might* clean up the resource. There are a number of circumstances in which they won't run at all. They are, by design, unreliable. For resources that need to be reliably cleaned up, you need to be manually disposing of them. There are also plenty of disposable resources that *don't* have finalizers. It is incorrect to assume that every single disposable resource must have implemented a finalizer, just as it's incorrect to assume that that finalizer must run. It is not "incorrect" for a disposable resource to not implement a finalizer. – Servy Aug 13 '15 at 14:38
  • @RonBeyer WIthout knowing the intent of the method, and the context of its use, that's rather hard to comment on. It seems that this method is created with the intent of cleaning up the object when it's done. Whether that actually makes sense for this particular operation is something that we don't really have enough information to comment on. – Servy Aug 13 '15 at 14:40
  • @RonBeyer Both `HttpResponseMessage` and `HttpContent` are correctly (where correctly means "as suggested by Microsoft on MSDN) done and can accept multiple `Dispose()` without any problem – xanatos Aug 13 '15 at 14:43
  • Maybe the `Dispose` was the wrong choice of an example, but if something somewhere else assumes that the `HttpResponseMessage` is not disposed and tries to access the `Content` property, it will except as trying to access a disposed object. I agree with Servy though, the context here is not enough to draw a conclusion from the example. – Ron Beyer Aug 13 '15 at 14:46