[SOLVED] C# BackgroundWorker OutOfRangeException

[tom982]

Hi guys,

I just found a bug in a project I'm working on and managed to fix it, but I don't understand what was wrong in the first place so I'm hoping someone can explain it for me. The app extracts Windows updates, checks for version numbers and logs them to a list view (which is also exported into a csv file at the end). This was the error I was seeing when processing most, but not all, of the updates in the folder:

Here's the source:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Diagnostics;
using System.Drawing;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace UpdateCatalog
{
    public partial class UpdateCatalog : Form
    {
        public UpdateCatalog()
        {
            InitializeComponent();
        }

        public string expand = Environment.GetFolderPath(Environment.SpecialFolder.Windows) + @"\system32\expand.exe";
        public string input = "";
        public int updatecount = 0;
        public int versioncount = 0;

        private void PreProcessing()
        {

        }

        private void PostProcessing()
        {
            this.Invoke((MethodInvoker)delegate
            {
                MessageBox.Show("Process finished! \r\n" + updatecount.ToString() + " updates processed, " + versioncount.ToString() + " version numbers found.", "Completed");
                btnRun.Enabled = true;
                panel1.Show();
            });

            CSV.ListViewToCSV(listView1, input + "\\results.csv", true);
        }

        private void btnRun_Click(object sender, EventArgs e)
        {
            progressBar1.Value = 0;
            updatecount = 0;
            versioncount = 0;
            listView1.Items.Clear();

            if (Directory.Exists(txtInput.Text))
            {
                input = txtInput.Text;
                btnRun.Enabled = false;
                panel1.Hide();
                PreProcessing();
                backgroundWorker1.RunWorkerAsync();
                btnCancel.Enabled = true;
            }
            else
            {
                MessageBox.Show("Please enter a valid folder path", "Error!");
            }
        }

        private void backgroundWorker1_DoWork(object sender, DoWorkEventArgs e)
        {
            string[] msufiles = Directory.GetFiles(input, "*.msu", System.IO.SearchOption.AllDirectories);
            this.Invoke((MethodInvoker)delegate
            {
                progressBar1.Maximum = msufiles.Length;
            });
            
            foreach (string file in msufiles)
            {
                string KB = Path.GetFileNameWithoutExtension(file);
                Regex rgx = new Regex(@"\bKB\d+\b", RegexOptions.IgnoreCase);

                foreach (Match match in rgx.Matches(KB))
                {
                    KB = match.ToString().ToUpper();
                    string output = input + "\\" + KB;
                    List<string> versions = new List<string>();

                    try
                    {
                        // Extract MSU
                        Directory.CreateDirectory(output);
                        ProcessStartInfo msustartInfo = new ProcessStartInfo(expand, @"-f:*" + " \"" + file + "\" \"" + output + "\"");
                        msustartInfo.WindowStyle = ProcessWindowStyle.Hidden;

                        Process msu = Process.Start(msustartInfo);
                        msu.WaitForExit(30000);

                        // Extract CAB
                        string[] files = Directory.GetFiles(output, "*");
                        string[] cabfile = Directory.GetFiles(output, "*.cab");
                        ProcessStartInfo cabstartInfo = new ProcessStartInfo(expand, @"-f:*" + " \"" + cabfile[0] + "\" \"" + output + "\"");
                        cabstartInfo.WindowStyle = ProcessWindowStyle.Hidden;
                        Process cab = Process.Start(cabstartInfo);
                        cab.WaitForExit(30000);

                        foreach (string deletion in files)
                        {
                            File.Delete(deletion);
                        }

                        updatecount++;

                        // Get list of folder names and manifest names
                        string[] folders = Directory.GetDirectories(output);
                        string[] manifests = Directory.GetFiles(output, "*.manifest");
                        Regex version = new Regex(@"\d+(?:\.\d+)+");
                        string versionnumbers = "";

                        foreach (string folder in folders)
                        {
                            string folderpath = new DirectoryInfo(folder).Name;
                            foreach (Match match2 in version.Matches(folderpath))
                            {
                                if (!(versionnumbers.Contains(match2.ToString())))
                                {
                                    versions.Add(match2.ToString());
                                }
                            }
                        }

                        foreach (string manifest in manifests)
                        {
                            string manifestname = Path.GetFileNameWithoutExtension(manifest);
                            foreach (Match match3 in version.Matches(manifest))
                            {
                                if (!(versionnumbers.Contains(match3.ToString())))
                                {
                                    versions.Add(match3.ToString());
                                }
                            }
                        }

                        versions = versions.Distinct().ToList();

                        string[] arr = new string[4];
                        ListViewItem itm;

                        arr[0] = KB;
                        arr[1] = versions[0];
                        arr[2] = versions[1];

                        itm = new ListViewItem(arr);

                        this.Invoke((MethodInvoker)delegate
                        {
                            listView1.Items.Add(itm);
                            listView1.Items[listView1.Items.Count - 1].EnsureVisible();
                        });
                    }
                    catch (Exception ex)
                    {
                        MessageBox.Show(ex.ToString());
                    }

                    versioncount = versioncount + versions.Count() - 1;
                }
                this.Invoke((MethodInvoker)delegate
                {
                    progressBar1.PerformStep();
                });
            }
            PostProcessing();
        }

        private void btnCancel_Click(object sender, EventArgs e)
        {
            if (backgroundWorker1.IsBusy)
            {
                backgroundWorker1.CancelAsync();
                
            }
            else
            {
                MessageBox.Show("Unable to cancel operation", "Error");
            }
        }
    }
}

The above source is fixed and doesn't produce the bug, but this bit:

                        string[] arr = new string[4];
                        ListViewItem itm;

                        arr[0] = KB;
                        arr[1] = versions[0];
                        arr[2] = versions[1];

                        itm = new ListViewItem(arr);

                        this.Invoke((MethodInvoker)delegate
                        {
                            listView1.Items.Add(itm);
                            listView1.Items[listView1.Items.Count - 1].EnsureVisible();
                        });

Used to be this:

                        this.Invoke((MethodInvoker)delegate
                        {
                            string[] arr = new string[4];
                            ListViewItem itm;

                            arr[0] = KB;
                            arr[1] = versions[0];
                            arr[2] = versions[1];

                            itm = new ListViewItem(arr);
                            listView1.Items.Add(itm);
                            listView1.Items[listView1.Items.Count - 1].EnsureVisible();
                        });

I needed to call invoke for updating the list view from the background worker but the other lines didn't need to be in there - why was it causing a problem though? Even though it caught exceptions on just about every update, the output was fine so it can't have been too disastrous whatever it is that it didn't like.

Tom

 

[AceInfinity]

That change wouldn't have affected your error at all. I would expect that the index for either:

listView1.Items

[plain]listView1.Items[listView1.Items.Count - 1].EnsureVisible();[/plain]

or...

versions

[plain]arr[1] = versions[0];
arr[2] = versions[1];[/plain]

Is negative or higher or equal to the total elements. You also have:

[plain]cabfile[0][/plain]

Being used without checking if a particular file was even found or not, in which case index [0] wouldn't exist. Bounds checking is basic handling for any arrays. Don't assume that you'll always have that index when it is not guaranteed.

 

[tom982]

Thanks Ace, you're exactly right - each of the updates that it failed for only had one version number in them and so versions[1] didn't exist for these. I guess it must have been pure coincidence that the sample updates I picked to test my 'fixed' version on had two version numbers in them, just my luck lol. I'll rewrite that code to use lists instead.

 

[AceInfinity]

Thanks Ace, you're exactly right - each of the updates that it failed for only had one version number in them and so versions[1] didn't exist for these. I guess it must have been pure coincidence that the sample updates I picked to test my 'fixed' version on had two version numbers in them, just my luck lol. I'll rewrite that code to use lists instead.

No need to change the container type, just check the total number of elements to make sure that you're upperbounded 0-based array index is less than the total elements.

 

[tom982]

Sorry, I should've made myself clearer as I said something completely unrelated to this problem, lol. As I don't know the number of distinct version numbers in each update, I thought it was better to use lists as they're dynamically sized.