Why does the data in a bitmap go out of scope?

Question

I am creating a save game manager for Skyrim and have run into an issue. When I create a SaveGame object by itself, the bitmap portion of the save works correctly. When I call that method in a loop, however, the bitmap takes on an erroneous value, mainly one that is similar to that of another save game.

TL;DR - Why is the listbox of my form displaying correct info for a character save except for the picture that is embedded? Rather than choosing the correct picture, it appears to select the last processed. How is the process different than when selected through an open file dialog?

---

Edit: Update - I looked into the bitmaps stored with each SaveGame object and found that during the creation of the SaveGames in the scanDirectoryForSaves is somehow messing it up. Is there an object scope issue with bitmaps and using a byte pointer that I'm not aware of?

---

Here is the code for my save game object's static factory:

public string Name { get; private set; }
    public int SaveNumber { get; private set; }
    public int PictureWidth { get; private set; }
    public int PictureHeight { get; private set; }
    public Bitmap Picture { get; private set; }
    public DateTime SaveDate { get; private set; }
    public string FileName { get; private set; }

    public static SaveGame ReadSaveGame(string Filename)
    {

        SaveGame save = new SaveGame();
        save.FileName = Filename;

        byte[] file = File.ReadAllBytes(Filename);

        int headerWidth = BitConverter.ToInt32(file, 13);
        save.SaveNumber = BitConverter.ToInt32(file, 21);
        short nameWidth = BitConverter.ToInt16(file, 25);

        save.Name = System.Text.Encoding.UTF8.GetString(file, 27, nameWidth);
        save.PictureWidth = BitConverter.ToInt32(file, 13 + headerWidth - 4);
        save.PictureHeight = BitConverter.ToInt32(file, 13 + headerWidth);
        save.readPictureData(file, 13 + headerWidth + 4, save.PictureWidth, save.PictureHeight);

        save.SaveDate = DateTime.FromFileTime((long)BitConverter.ToUInt64(file, 13 + headerWidth - 12));

        return save;
    }

    private  void readPictureData(byte[] file, int startIndex, int width, int height)
    {
        IntPtr pointer = Marshal.UnsafeAddrOfPinnedArrayElement(file, startIndex);
        Picture =  new Bitmap(width, height, 3 * width, System.Drawing.Imaging.PixelFormat.Format24bppRgb, pointer);
    }

On my form, I use a method to read all the save files in a certain directory, create SaveGame objects out of them, and store them in a dictionary based on the character name.

private Dictionary<string, List<SaveGame>> scanDirectoryForSaves(string directory)
    {
        Dictionary<string, List<SaveGame>> saves = new Dictionary<string, List<SaveGame>>();
        DirectoryInfo info = new DirectoryInfo(directory);

        foreach (FileInfo file in info.GetFiles())
        {
            if (file.Name.ToLower().EndsWith(".ess") || file.Name.ToLower().EndsWith(".bak"))
            {
                string filepath = String.Format(@"{0}\{1}", directory, file.Name);
                SaveGame save = SaveGame.ReadSaveGame(filepath);

                if (!saves.ContainsKey(save.Name))
                {
                    saves.Add(save.Name, new List<SaveGame>());
                }
                saves[save.Name].Add(save);
            }
        }

        foreach (List<SaveGame> saveList in saves.Values)
        {
            saveList.Sort();
        }

        return saves;
    }

I add the keys to a list box. When a name is selected on the list box, the latest save for the character is displayed on the form. The name, date, and other fields are correct for each character, but the bitmap is a variation of a certain characters save game picture.

I am calling the same method to update the form fields in both selecting a save from an open file dialog as well as the listbox.

private void updateLabels(SaveGame save)
    {
        nameLabel.Text = "Name: " + save.Name;
        filenameLabel.Text = "File: " + save.FileName;
        saveNumberLabel.Text = "Save Number: " + save.SaveNumber;

        saveDateLabel.Text = "Save Date: " + save.SaveDate;

        saveGamePictureBox.Image = save.Picture;
        saveGamePictureBox.Image = ScaleImage(
            saveGamePictureBox.Image, saveGamePictureBox.Width, saveGamePictureBox.Height);
        saveGamePictureBox.Invalidate();
    }

Answer 1

When you create a Bitmap using the constructor that takes an IntPtr, the IntPtr must point to a block of memory that remains valid for the lifetime of the Bitmap object. You are responsible for ensuring that the block of memory doesn't get moved or deallocated.

However, your code is passing an IntPtr that points to file, which is a managed byte array. Because nothing references file after ReadSaveGame returns, the garbage collector is free to reclaim the memory and reuse it for the next file. The result: corrupted bitmaps.

Although you could fix this problem by pinning the array in memory with GCHandle, it's probably easier and safer to just let the Bitmap manage its own memory. First create an empty Bitmap, then set its bits:

private void readPictureData(byte[] file, int startIndex, int width, int height)
{
    Bitmap bitmap = new Bitmap(width, height, PixelFormat.Format24bppRgb);
    BitmapData data = bitmap.LockBits(
        new Rectangle(0, 0, width, height),
        ImageLockMode.WriteOnly, PixelFormat.Format24bppRgb);
    Marshal.Copy(file, startIndex, data.Scan0, width * height * 3);
    bitmap.UnlockBits(data);
    Picture = bitmap;
}