1

I'm trying to use streams to progressively display a jpeg as it loads. This used to work fine, but this morning I tried running my code and now no images can be loaded because of this error. The relevant code is as follows:

using (WebClient wc = new WebClient())
using (Stream streamRemote = wc.OpenRead(url))
using (Stream streamLocal = new MemoryStream((int)fileSize))
{
    int byteSize = 0;
    byte[] buffer = new byte[fileSize];

    while ((byteSize = streamRemote.Read(buffer, 0, buffer.Length)) > 0)
    {
        streamLocal.Write(buffer, 0, byteSize); // Error is here.
        bytesDownloaded += byteSize;

        int progressPercentage = (int)(bytesDownloaded / buffer.Length) * 100;

        if (progressPercentage % 10 == 0)
        {
            imageLoader.ReportProgress(progressPercentage, streamLocal);
        }
    }
}

// Exception thrown: 'System.ObjectDisposedException' in mscorlib.dll
// An exception of type 'System.ObjectDisposedException' occurred in mscorlib.dll but was not handled in user code
// Cannot access a closed Stream.

After using Console.WriteLine at the end of the that final using statement (after the while loop), I've found that the code seems to run through the loop a couple times before throwing that exception.

I don't understand why I would be trying to access a closed stream when the code is clearly happening within the using statement that the stream is declared in. I also don't understand why it worked the other day and doesn't now. Most of the code for this comes from here, so the rest of my method can be found there. Mine isn't much different apart from some variable name changes and other small changes. Can anyone help fix this?

Edit: My _ProgressChanged event:

private void ImageLoader_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    if (!fileFailed)
    {
        Dispatcher.BeginInvoke(System.Windows.Threading.DispatcherPriority.Normal,
        new Action(delegate ()
        {
            try
            {
                using (MemoryStream stream = e.UserState as MemoryStream)
                {
                    BitmapImage bmp = new BitmapImage();
                    bmp.BeginInit();
                    using (MemoryStream ms = new MemoryStream(stream.ToArray())) bmp.StreamSource = ms;
                    bmp.EndInit();

                    img_CurrentImage.Source = bmp;
                }
            }
            // A few catch statements here - none of the exceptions here are being thrown anyway so I'll omit the catch statements
        }));
    }
}
Athied
  • 25
  • 1
  • 5
  • Is you `_ProgressChanged` handler identical to that shown in the other answer? I'm a bit wary of the way it's being used to "smuggle" the `MemoryStream` into another context. If it's misused in that other context, that could be the source of the issue. – Damien_The_Unbeliever Sep 15 '18 at 06:22
  • @Damien_The_Unbeliever It's almost identical, but I'll edit it into the post anyway. – Athied Sep 15 '18 at 06:36
  • Seems like you do something with Treading in the ImageLoader_ProgressChanged. if this is a different tread, I understand the error. I am not familair with Dispatcher so see this as a possible hint. – Aldert Sep 15 '18 at 06:49

1 Answers1

4

As I suspected, it's because you're misusing the MemoryStream in your ProgressChanged handler.

This line:

using (MemoryStream stream = e.UserState as MemoryStream)

Is taking the stream object that was passed to ProgressChanged() and will later on in the delegate Dispose of it. But that's exactly the same stream object that you're wondering how it got disposed inside your DoWork method.

That stream doesn't "belong" to the ProgressChanged handler. It shouldn't be disposing of it.

It's somewhat subtle, but in the linked question, the only thing that's done with the passed in stream is to access its ToArray method. Which you need to also be careful to do since, because ProgressChanged (and Dispatcher.BeginInvoke) is asynchronous, you could easily be dealing with an already disposed object in this handler (but ToArray is safe to call on disposed MemoryStreams)

You might also consider extracting the byte[] array from the MemoryStream inside your DoWork method and making that the passed state instead of the MemoryStream. That would make it far less prone to misuse both now and in any future edits to this code.

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • Ah, this was it. Thank you! Could you elaborate on your last paragraph a little though, please? Would I pass the byte[] array into the _ProgressChanged event and then make a new MemoryStream with that array, or what? – Athied Sep 15 '18 at 07:50
  • @Athied - in `DoWork`, have `imageLoader.ReportProgress(progressPercentage, streamLocal.ToArray());` instead. Then yes, pass that in to the existing call of `ProgressChanged` that;s creating a new `MemoryStream`. – Damien_The_Unbeliever Sep 15 '18 at 09:10