Exception when updating UI element from some work done on separate thread

Question

I have a UI application that simply recursively search for particular file type and display the results in a list box. However it is causing the classic UI freeze problem so I am trying to do the search in separate thread and the update the listbox in UI thread. I have two solutions, one of these is causing an exception while other is working great. Problem is I don’t understand why solution #1 is throwing exception.

Solution #1
This throws IndexOutOfRangeException: Index was outside the bounds of the array at the call to this.BeginInvoke((Action)(() => { this.lsResult.Items.Add(files[startingIndex].ToString()); }));

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
    }

    private void btnSearch_Click(object sender, EventArgs e)
    {

        this.lsResult.Items.Clear();
        this.Cursor = Cursors.WaitCursor;

        new Thread(BeginSearch).Start();

        this.Cursor = Cursors.Default;
    }


    private void BeginSearch()
    {
        string searchPath = this.textBox1.Text; // C:\*.txt
        RecurseDirectory(searchPath);

    }
    private void RecurseDirectory(string searchPath)
    {
        string directory = Path.GetDirectoryName(searchPath);
        string search = Path.GetFileName(searchPath);

        if (directory == null || search == null)
        {
            return;
        }

        string[] files = Directory.GetFiles(directory, search);
        int startingIndex = 0;
        while (startingIndex < files.Length)
        {


            //IndexOutOfRangeException: Index was outside the bounds of the array.

            this.BeginInvoke((Action)(() =>
            {
                this.lsResult.Items.Add(files[startingIndex].ToString());
            }));
            Interlocked.Increment(ref startingIndex);
        }

        string[] directories = Directory.GetDirectories(directory);
        foreach (string d in directories)
        {
            RecurseDirectory(Path.Combine(d, search));
        }
    }

}

Solution #2
This works great.

public partial class Form1 : Form
{
    private delegate void FileListDelegate(string[] files);
    private FileListDelegate _FileListDelegate;
    public Form1()
    {
        InitializeComponent();
        _FileListDelegate = new FileListDelegate(ShowFileNames);
    }

    private void btnSearch_Click(object sender, EventArgs e)
    {

        this.lsResult.Items.Clear();
        this.Cursor = Cursors.WaitCursor;

        new Thread(BeginSearch).Start();

        this.Cursor = Cursors.Default;
    }


    private void BeginSearch()
    {
        string searchPath = this.textBox1.Text;
        RecurseDirectory(searchPath);

    }
    private void RecurseDirectory(string searchPath)
    {
        string directory = Path.GetDirectoryName(searchPath);
        string search = Path.GetFileName(searchPath);

        if (directory == null || search == null)
        {
            return;
        }

        string[] files = Directory.GetFiles(directory, search);
        this.BeginInvoke(_FileListDelegate, new object[] { files });

        string[] directories = Directory.GetDirectories(directory);
        foreach (string d in directories)
        {
            RecurseDirectory(Path.Combine(d, search));
        }
    }

    void ShowFileNames(string[] files)
    {
        foreach(string file in files)
            this.lsResult.Items.Add(file.ToString());
    }

}

Both solutions look very same to me and I don’t understand why #1 is throwing exception.

Answer 1

You're closing over the variable startingIndex in your first solution. It's important to realize that closures close over variables, not over values. The method will use the value of startingIndex when the delegate is eventually executed, which could be at some point in the distant future, instead of using the value of that variable right now. You're constantly incrementing startIndex until it's set to a value past the end of the array, hence your index out of bounds errors when you end up executing code using that variable after all of those increments.

The smallest code change would be to simply take a copy of the variable inside of the loop that won't change ever. Then close over that variable.

int index = startingIndex;
this.BeginInvoke((Action)(() =>
{
    this.lsResult.Items.Add(files[index].ToString());
}));

Also note that you can greatly simplify your code by having the file system recursively traverse itself. Just use Directory.GetFiles(searchPath, "*.*", SearchOption.AllDirectories) instead of explicitly handling the recursion yourself, then you can just loop through those files using a foreach and add each file to the UI.