0

We have an application which contains a form that interfaces with a camera. The user can control the camera from the form, take pictures with it, display it in a WinForms PictureBox, open another form to edit the picture that is being displayed or save that picture to disk. Our customers have been complaining about intermittent AccessViolationExceptions. After the better part of an afternoon, I found a reliable way for me to reproduce the issue on my development environment by forcing a GC.Collect before saving the PictureBox.Image to a MemoryStream.

We populate our PictureBox control like this:

int w = videoInfoHeader.BmiHeader.Width;
int h = videoInfoHeader.BmiHeader.Height;
int stride = w * 3;

GCHandle handle = GCHandle.Alloc(savedArray, GCHandleType.Pinned);
long scan0 = (long)handle.AddrOfPinnedObject();
scan0 += (h - 1) * stride;  //image is upside down, so start at the bottom (I guess bottom-up bitmaps still scan left-to-right?)
Bitmap b = new Bitmap(w, h, -stride, PixelFormat.Format24bppRgb, (IntPtr)scan0);
handle.Free();
Image old = pictureBog.Image;
pictureBox.Image = b;
if (old != null)
    old.Dispose();
//Show picturebox, and let user use the form

When the user wants to edit the photo, we pull out the bitmap as a MemoryStream formatted as a Jpeg (I don't know why):

//GC.Collect()
using (MemoryStream stream = new MemoryStream())
{
    pictureBox.Image.Save(stream, ImageFormat.Jpeg);  //AccessViolationException here if GC.Collect() is uncommented.
    byte[] pic = stream.ToArray();
    //Do stuff with pic and open editor form
}

I noted that from Randomly occurring AccessViolationException in GDI+ there are issues where the GC moves stuff out from under the unmanaged code, so you have to "pin" your managed objects to prevent this from happening. I see that we are doing this when we populated the PictureBox, but not when we are pulling the image out of the PictureBox. I understand needing to "pin" the source byte array, as a byte array is just a dumb array. However, I don't even know what I would try to "pin" when saving the image - the MemoryStream? I would imagine that Bitmap.Save(Stream) would be smart enough to handle that automagically if it were needed. Plus the buffer will need to be reallocated as it fills up anyways, so pinning doesn't make much sense to me. Anyone have any idea what is causing the bad memory access here?

I suppose the worst case scenario is we avoid using the PictureBox when pulling out image data, hold onto the original byte array, build the Bitmap from that and save from the fresh copy. This seems to work, but I don't know if this is just coincidence and the real problem is still there.

Taedrin
  • 445
  • 1
  • 4
  • 16
  • 2
    Have a look at the MSDN documentation for the Bitmap constructor. There is a remark: "The caller is responsible for allocating and freeing the block of memory specified by the scan0 parameter, however, the memory should not be released until the related Bitmap is released." You are releasing the pinned memory in your code, but use the bitmap anymore. If you perform a garbage collection immediately, you get the corresponding exception immediately, too. Otherwise, the exception may occur anytime in the future, when the GC decides to run. – KBO Nov 10 '17 at 09:52

1 Answers1

0

A much safer way to make an image from a byte array is to first make the image itself with the simple new Bitmap(width, height, pixelformat) constructor, then opening its backing data with LockBits, and then copying your data into the Scan0 pointer it exposes, scanline by scanline, using Marshal.Copy. That way, you are never messing with unmanaged pointers that could cause problems. The only pointer that gets involved is in the specifically locked memory that is reserved by LockBits, and after unlockBits is called, that is once again safely made part of the internal image data.

The code can be found here:

A: Why must "stride" in the System.Drawing.Bitmap constructor be a multiple of 4?

(The answer is the same, but the question is vastly different, so I don't think there's much use in marking this as duplicate)

Note that handling for upside-down stride is included in the method.

Nyerguds
  • 5,360
  • 1
  • 31
  • 63