Using Threads and .Invoke() and controls still remain inactive - C#

I am trying to populate a text box with some data, namely the names of several instruments a line at a time.

I have a class that will generate and return a list of instruments, I then iterate through the list and append a new line to the text box after each iteration.

Starting the Thread:

private void buttonListInstruments_Click(object sender, EventArgs e)
        {
            if (ins == null)
            {
                ins = new Thread(GetListOfInstruments);
                ins.Start();
            }
            else if (ins != null)
            {
                textBoxLog.AppendText("Instruments still updating..");开发者_如何转开发
            }

        }

Delegate to update textbox:

public delegate void UpdateLogWithInstrumentsCallback(List<Instrument> instruments);

private void UpdateInstruments(List<Instrument> instruments)
        {
            textBoxLog.AppendText("Listing available Instruments...\n");

            foreach (var value in instruments)
            {
                textBoxLog.AppendText(value.ToString() + "\n");
            }
            textBoxLog.AppendText("End of list. \n");

            ins = null;
        }

Invoking the control:

private void GetListOfInstruments()
        {
            textBoxLog.Invoke(new UpdateLogWithInstrumentsCallback(this.UpdateInstruments),
                new object[] { midiInstance.GetInstruments() });
        }

Note: GetInstruments() returns a List of type Instrument.

I am implementing therads to try to keep the GUI functional whilst the text box updates. For some reason the other UI controls on the WinForm such as a seperate combo box remain inactive when pressed until the text box has finished updating.

Am I using threads correctly?

Thanks.

You haven't accomplished anything, the UpdateInstruments() method still runs on the UI thread, just like it did before. Not so sure why you see such a long delay, that must be a large number of instruments. You can possibly make it is less slow by first appending all of them into a StringBuilder, then append its ToString() value to the TextBox. That cuts out the fairly expensive Windows call.

I would recommend using a SynchronizationContext in general:

From the UI thread, e.g. initialization:

// make sure a SC is created automatically
Forms.WindowsFormsSynchronizationContext.AutoInstall = true;
// a control needs to exist prior to getting the SC for WinForms
// (any control will do)
var syncControl = new Forms.Control();
syncControl.CreateControl();
SyncrhonizationContext winformsContext = System.Threading.SynchronizationContext.Current;

Later on, from any thread wishing to post to the above SC:

// later on -- no need to worry about Invoke/BeginInvoke! Whoo!
// Post will run async and will guarantee a post to the UI message queue
// that is, this returns immediately
// it is OKAY to call this from the UI thread or a non-UI thread
winformsContext.Post(((state) => ..., someState);

As others have pointed out, either make the UI update action quicker (this is the better method!!!) or separate it into multiple actions posted to the UI queue (if you post into the queue then other message in the queue won't be blocked). Here is an example of "chunking" the operations into little bit of time until it's all done -- it assumes UpdateStuff is called after the data is collected and not necessarily suitable when the collection itself takes noticeable time. This doesn't take "stopping" into account and is sort of messy as it uses a closure instead of passing the state. Anyway, enjoy.

void UpdateStuff (List<string> _stuff) {
    var stuff = new Queue<string>(_stuff); // make copy
    SendOrPostCallback fn = null; // silly so we can access in closure
    fn = (_state) => {
        // this is in UI thread
        Stopwatch s = new Stopwatch();
        s.Start();
        while (s.ElapsedMilliseconds < 20 && stuff.Count > 0) {
          var item = stuff.Dequeue();
          // do stuff with item
        }
        if (stuff.Count > 0) {
          // have more stuff. we may have run out of our "time-slice"
          winformsContext.Post(fn, null);
        }
    };
    winformsContext.Post(fn, null);
}

Happy coding.

Change this line:

textBoxLog.Invoke(new UpdateLogWithInstrumentsCallback(this.UpdateInstruments),
                new object[] { midiInstance.GetInstruments() });

with this:

textBoxLog.BeginInvoke(new UpdateLogWithInstrumentsCallback(this.UpdateInstruments),
                new object[] { midiInstance.GetInstruments() });

You are feeding all instruments into the textbox at once rather then one-by-one in terms of threading. The call to Invoke shall be placed in the for-loop and not to surround it.

nope, you start a thread, and then use invoke, which basically means you are going back to the UI thread to do the work... so your thread does nothing!

You might find that it's more efficient to build a string first and append to the textbox in one chunk, instead of line-by-line. The string concatenation operation could then be done on the helper thread as well.