CODEWITHSUNDEEP
c#.netdispose
Mike Flynn
Mike Flynn

Reputation: 24325

Dispose of File.Open or StreamContent

Is the below line for content.Add going to leave an object from being disposed correctly? If so what is the correct way to handle this.

 public string UploadGameToWebsite(string filename, string url, string method = null)
        {
            var client = new HttpClient();
            var content = new MultipartFormDataContent();
            content.Add(new StreamContent(File.Open(filename, FileMode.Open, FileAccess.Read)), "Game", "Game.txt");
            var task = client.PutAsync(url, content);
            var result = task.Result.ToString();
            return result;
        }

Upvotes: 3

Views: 6905

Answers (2)

Igor
Igor

Reputation: 62308

  1. Make your method asynchronous if you are going to be calling an async operation in your method.
  2. Dispose your file stream and also the client. In the example below by disposing the StreamContent it also disposes the underlying FileStream.
  3. I prefer a finally block for disposing multiple disposable objects, its also perfectly fine to have nested using statements.
  4. Not sure why you are returning the ToString on the HttpResponseMessage, maybe status code would be more useful or see if StatusCode = 200 and return boolean (true/false)?

Code:

public async Task<string> UploadGameToWebsiteAsync(string filename, string url, string method = null)
{
    HttpClient client = null;
    StreamContent fileStream = null;
    try
    {
        client = new HttpClient();
        fileStream = new StreamContent(System.IO.File.Open(filename, FileMode.Open, FileAccess.Read))
        var content = new MultipartFormDataContent();
        content.Add(fileStream, "Game", "Game.txt");
        HttpResponseMessage result = await client.PutAsync(url, content);
        return result.ToString();
    }
    finally 
    {
        // c# 6 syntax
        client?.Dispose();
        fileStream?.Dispose(); // StreamContent also disposes the underlying file stream
    }
}

Code version #2 using using blocks.

public async Task<string> UploadGameToWebsiteAsync(string filename, string url, string method = null)
{
    using (var client = new HttpClient())
    {
        using (var fileStream = new StreamContent(System.IO.File.Open(filename, FileMode.Open, FileAccess.Read)))
        {
            var content = new MultipartFormDataContent();
            content.Add(fileStream, "Game", "Game.txt");
            HttpResponseMessage result = await client.PutAsync(url, content);
            return result.ToString();
        }
    }
}

Upvotes: 6

Mikael Nitell
Mikael Nitell

Reputation: 1129

Yes it will. File.Open returns a stream that should be disposed. The easiest way to do this would be to use a "using" block, like this:

 public string UploadGameToWebsite(string filename, string url, string method = null)
    {
        var client = new HttpClient();
        var content = new MultipartFormDataContent();
        using (var fileStream = File.Open(filename, FileMode.Open, FileAccess.Read))
        {
            content.Add(new StreamContent(fileStream), "Game", "Game.txt");
            var task = client.PutAsync(url, content);
            var result = task.Result.ToString();            
            return result;
        }
    }

Upvotes: 1

Related Questions