14
public ActionResult CustomChart(int reportID)
{
    Chart chart = new Chart();

    // Save the chart to a MemoryStream
    var imgStream = new MemoryStream();
    chart.SaveImage(imgStream);
    imgStream.Seek(0, SeekOrigin.Begin);

    // Return the contents of the Stream to the client
    return File(imgStream, "image/png");
}

I am accustomed to using the 'using' statement in conjuction with MemoryStreams. Is this a scenario where the 'using' statement is not necessary? Or is it valid to call return inside of a 'using' statement?

EDIT:

For my purposes I have found that the introduction of a 'using' statement does NOT work (throws an ObjectDisposedException). Here's what I'm doing with it client-side:

$('#ReportTest').bind('load', function () {
                        $('#LoadingPanel').hide();
                        $(this).unbind('load');
                    }).bind('error', function () {
                        $('#LoadingPanel').hide();
                        $(this).unbind('error');
                    }).attr('src', '../../Chart/CustomChart?ReportID=' + settings.id);
knocte
  • 16,941
  • 11
  • 79
  • 125
Sean Anderson
  • 27,963
  • 30
  • 126
  • 237
  • 2
    It's always a good idea to use the 'using' statement when dealing with classes that implement IDisposable, whether or not you believe ASP.NET is going to clean up after you. – George Stocker Jan 10 '12 at 17:54
  • 1
    what does `File(stream, string)` do with the stream? typically the object which created the stream should also dispose of the stream. in that case you would be responsible disposing of the stream. – Jason Meckley Jan 10 '12 at 17:55
  • @GeorgeStocker would the `Dispose()` method even be called? I would think after `return` is called it wouldn't complete the `using` block. –  Jan 10 '12 at 17:59
  • Possible dupe'ish http://stackoverflow.com/questions/662773/returning-in-the-middle-of-a-using-block – Craig Jan 10 '12 at 18:01
  • 1
    @Shark The dispose is called in a `finally`, so yes - it'll get called. – vcsjones Jan 10 '12 at 18:06
  • Create a Stream descendant that writes out messages using Debug.WriteLine to verify what happens. – Lasse V. Karlsen Jan 10 '12 at 18:08
  • 1
    It is good practice to `Dispose` disposable objects. But for `MemoryStream` the only thin that will do is prevent you from reading/writing to it. So you current implementation works fine. – Magnus Jan 10 '12 at 18:12

4 Answers4

25

Does a MemoryStream get disposed of automatically when returning it as an ActionResult?

Yes, MVC (at least version 3) will clean it up for you. You can take a look at the source of the WriteFile method in FileStreamResult:

protected override void WriteFile(HttpResponseBase response) {
    // grab chunks of data and write to the output stream
    Stream outputStream = response.OutputStream;
    using (FileStream) {
        byte[] buffer = new byte[_bufferSize];

        while (true) {
            int bytesRead = FileStream.Read(buffer, 0, _bufferSize);
            if (bytesRead == 0) {
                // no more data
                break;
            }

            outputStream.Write(buffer, 0, bytesRead);
        }
    }
}

The line using (FileStream) { will place the Stream in a using block, thus Disposing of it when it has written the contents to the Http Response.

You can also verify this behavior by creating a dummy stream that does this:

public class DummyStream : MemoryStream
{
    protected override void Dispose(bool disposing)
    {
        Trace.WriteLine("Do I get disposed?");
        base.Dispose(disposing);
    }
}

So MVC will dispose it.

vcsjones
  • 138,677
  • 31
  • 291
  • 286
2

A MemoryStream is not necessary in this situation. You can avoid it by creating a Custom ActionResult like this :

public class ChartResult : ActionResult
{
    private Chart _chart;

    public ChartResult(Chart chart)
    {
        if (chart == null)
            throw new ArgumentNullException("chart");
        _chart = chart;
    }

    public override void ExecuteResult(ControllerContext context)
    {
        if (context == null)
            throw new ArgumentNullException("context");

        HttpResponseBase response = context.HttpContext.Response;
        response.ContentType = "image/png";
        response.BufferOutput = false;

        _chart.ImageType = ChartImageType.Png;
        _chart.SaveImage(response.OutputStream);
    }
}
Kalten
  • 4,092
  • 23
  • 32
  • Actually pretty nice solution! Seems like it'd save 1 buffer copy too. (May depend on how big a deal it is to keep `Chart` alive that long though) –  Nov 11 '13 at 16:36
2

Sean: Do NOT use 'using' as it will Dispose the object. Leaving MVC accessing a Disposed object. Hence the exception(server error) you experienced is certainly an ObjectDisposedException. The WriteFile function previously posted Disposes the object for you.

Steven Licht
  • 760
  • 4
  • 5
-3

The following is valid code that disposes of the stream. If enclosed in a using block the MemoryStream.Dispose() method will be called automatically when returning.

public ActionResult CustomChart(int reportID)
{
    Chart chart = new Chart();

    using (var imgStream = new MemoryStream()) {
        chart.SaveImage(imgStream);
        imgStream.Seek(0, SeekOrigin.Begin);
        return File(imgStream, "image/png");
    }
}

You can achieve the same result by putting the object inside a try block and then calling Dispose in a finally block. In fact, according to the MSDN documentation this is how the using statement is translated by the compiler. And in a try..finally block the finally will always be executed even when try exits through return.

The compiler will translate the using block to the following:

MemoryStream imgStream = new MemoryStream();

try 
{
    chart.SaveImage(imgStream);
    imgStream.Seek(0, SeekOrigin.Begin);
    return File(imgStream, "image/png");
}
finally
{
    if (imgStream != null) 
        ((IDisposable)imgStream).Dispose();
}
Dennis Traub
  • 50,557
  • 7
  • 93
  • 108
  • 5
    Won't that yank the stream out from underneath the File object? – Lasse V. Karlsen Jan 10 '12 at 18:03
  • 1
    @LasseV.Karlsen That is what I am wondering, too. And if it doesn't... why does it not? :) – Sean Anderson Jan 10 '12 at 18:04
  • +1 for comment @SeanAnderson I'd like to see the documentation on this myself. – Craig Jan 10 '12 at 18:06
  • @LasseV.Karlsen Yes, it will. – Dennis Traub Jan 10 '12 at 18:06
  • If the stream is processed directly in the `File` constructor there should not be a problem. – Magnus Jan 10 '12 at 18:07
  • 1
    File is not a constructor here, it is a method on the controller. – Lasse V. Karlsen Jan 10 '12 at 18:12
  • @LasseV.Karlsen right, still depends on what that method actually does. – Magnus Jan 10 '12 at 18:13
  • So have we established that this works or not? If it doesn't, I'm going to close this question as too localized. – Lasse V. Karlsen Jan 10 '12 at 18:15
  • 2
    I just tried it out in my application. The introduction of a using statement does NOT work for my application. I have an internal server error when trying to render the image file. – Sean Anderson Jan 10 '12 at 18:31
  • 1
    I do not recommend doing this: it will close the stream before returning it and you will get an error. Suppose you are returning a stream with a pdf to be displayed on a dialog, you will get "cannot access closed stream error" if you put the return inside a using clause\ – victor Sep 23 '14 at 19:33
  • 1
    Just throwing my $.02 in that this is how I had it implemented thinking it should be this way and it totally doesn't work. It does indeed yank the stream out from File(..) before it has a chance to use it. I googled "will file dispose the stream? mvc" to verify (to verify what I suspected after a bit more testing) and this is what I was led to. – claudekennilol Jul 17 '15 at 14:15