Treeview Async Problems

Question

I am having a problem with a treeview where I try to add a subnode based upon the Node.Text Index (I have also tried this based upon the int index - to no avail). This works wonderfully when ran synchronously. However I run the exact same thing Async (backgroundWorker) it throws an unhandled ArgumentOutOfRange Exception. Another odd part is that I've attempted to catch this Exception in two different areas. see code:

 using (Microsoft.Win32.RegistryKey key = Registry.LocalMachine.OpenSubKey(registry_key))
        {
            int x = 0;
            foreach (string subkey_name in key.GetSubKeyNames())
            {
                using (RegistryKey subkey = key.OpenSubKey(subkey_name))
                {
                    foreach (string a in (string[])subkey.GetValue("Users", ""))
                    {
                        User u = new User(a);
                        usrs.addUser(new User(a));
                        wgs.addUserToWorkgroup(subkey_name, a);
                        usrs.AddWorkGroupToUser(subkey_name, a);
                        int trycount = 0;
                    TryAgain:
                        try
                        {
                            //here is where the exception occurs
                            ExecuteSecure(() => treeView1.Nodes[subkey_name].Nodes.Add(a, a));
                        }
                        catch (ArgumentOutOfRangeException)//This does not catch it.
                        {
                            trycount++;
                            if (trycount < 100)
                            {
                                goto TryAgain; //b/c I cannot catch it this never happens...
                            }
                        }

                    }
                }

                x++;
                //System.Threading.Thread.Sleep(2);
                //As you can see I've tried to let the tread sleep to resolve this 
                //- it will get a little farther but still eventually bomb out.
            }
        }

Here is the ExecuteSecure code (https://stackoverflow.com/a/8021020/1387186)

 private void ExecuteSecure(Action a)
    {
        o = new object();
        try
        {
            if (InvokeRequired)
            {
                lock (o)
                {
                    BeginInvoke(a);
                }
            }
            else
                a();
        }
        catch (Exception) //again **sigh** this does not catch the error
        { }

Answer 1

You have several problems.

• You are closing over the loop variable.
• Your lock is pointless because you use a different instance each time.
• Your lock is pointless because no other threads compete for it.
• Your new solution of using Invoke cause the UI and worker threads to ping-pong back and forth and as such your worker thread now became useless.

Anyone who monitors my answers on here knows how I feel about these marshaling techniques (like Invoke). It can be (and usually is) the worst method of updating the UI. In fact, it is so overused that it has gotten to the point where I think it can be considered a form of cargo cult programming.

What you should do is have your worker thread publish the data required to update the TreeView into a queue and then have your UI thread pull it out via a System.Windows.Forms.Timer on an interval that works best for you. Here is what it would look like.

public class YourForm : Form
{
  private sealed class YourData
  {
    public string SubKeyName { get; set; }
    public string Value { get; set; }
  }

  private ConcurrentQueue<YourData> queue = new ConcurrentQueue<YourData>();

  private void StartTreeViewUpdate_Click(object sender, EventArgs args)
  {
    Task.Factory.StartNew(WorkerThread);
    TreeViewUpdateTimer.Enabled = true;
  }

  private void TreeViewUpdateTimer_Tick(object sender, EventArgs args)
  {
    // Update in batches of 100 (or whatever) so that the UI stays
    // responsive.
    treeView1.BeginUpdate();
    for (int i = 0; i < 100; i++)
    {
      YourData value = null;
      if (queue.TryDequeue(value) && value != null)
      {
        treeView1.Nodes[value.SubKeyName].Nodes.Add(value.Value, value.Value)
      }
      else
      {
        // We're done.
        TreeViewUpdateTimer.Enabled = false;
        break;
      }
    }
    treeView1.EndUpdate();
  }

  private void WorkerThread()
  {
    using (RegistryKey key = Registry.LocalMachine.OpenSubKey(registry_key))
    {
      foreach (string subkey_name in key.GetSubKeyNames())
      {
        using (RegistryKey subkey = key.OpenSubKey(subkey_name))
        {
          foreach (string a in (string[])subkey.GetValue("Users", ""))
          {
            User u = new User(a);
            usrs.addUser(new User(a));
            wgs.addUserToWorkgroup(subkey_name, a);
            usrs.AddWorkGroupToUser(subkey_name, a);
            var data = new YourData();
            data.SubKeyName = subkey_name;
            data.Value = a;
            queue.Enqueue(data);
          }
          queue.Enqueue(null); // Indicate that queueing is done.
        }
      }
    }
  }
}

Answer 2

In case the comments are obvious, do this:

 var uncapturedSubKey_name = subkey_name;
 ExecuteSecure(() => treeView1.Nodes[uncapturedSubKey_name].Nodes.Add(a, a));

The lambda is capturing the the other variable and isn't executing until after the loop is done (i.e. the last subkey_name...

Answer 3

I was able to resolve this like so:

By changing the BeginInvoke() to Invoke() I also changed the scope of O so it actually locks.

private void ExecuteSecure(Action a)
{

    try
    {
        if (InvokeRequired)
        {
            lock (o)
            {
                Invoke(a);
            }
        }
        else
            a();
    }
    catch (Exception) //again **sigh** this does not catch the error
    { }