Asynchronous Writes To Textbox in C# are Overwritt

Question

I have an application wherein two threads write asynchronously to a single textbox. It works, except that the second thread to write to the textbox overwrites the line that the first thread just wrote. Any thoughts or insight into the problem would be greatly appreciated. I am using Microsoft Visual C# 2008 Express Edition. Thanks.

  delegate void SetTextCallback(string text);

  private void SetText(string text)
  {
     this.textBox1.Text += text;
     this.textBox1.Select(textBox1.Text.Length, 0);
     this.textBox1.ScrollToCaret();
  }

  private void backgroundWorkerRx_DoWork(object sender, DoWorkEventArgs e)
  {
     string sText = "";

     // Does some receive work and builds sText

     if (textBox1.InvokeRequired)
     {
        SetTextCallback d = new SetTextCallback(SetText);
        this.Invoke(d, new object[] { sText });
     }
     else
     {
        SetText(sText);
     }
  }

Answer 1

EDIT: This might not solve the problem, but you might want to handle the ProgressChanged event of the BackgroundWorkers and set the text there.

For example:

void backgroundWorker_ProgressChanged(object sender, ProgressChangedEventArgs e) {
    SetText((string)e.UserState);
}  //Make this method handle the RunWorkerCompleted for both workers

//In DoWork:
    worker.ReportProgress(0, sText);

ProgressChanged is fired on the UI thread, so you don't need to call Invoke.

By the way, you should probably rename SetText to AppendText to make the code clearer.
Also, you can use the built-in delegate Action<String> instead of making your own SetTextCallback delegate type.

EDIT: Also, you should probably move the InvokeRequired check to SetText.

For example:

private void AppendText(string text) {
    if(textBox1.InvokeRequired) {
        textBox1.Invoke(new Action<string>(AppendText), text);
        return;
    }
    this.textBox1.AppendText(text);
    this.textBox1.SelectionStart = textBox1.TextLength;
    this.textBox1.ScrollToCaret();
}

Answer 2

I agree with SLaks that you should be using the BackgroundWorker more properly. But to "fix" the code supplied, one issue is with the Invoke call... invocation needs to call upon the same method checking for requirement as to align the thread to the form's creator. I typically do something similar to the following (the argument usage may not be compilable but the rest is fine). Honestly, there should most likely only need to be one handler since only one thread can write at a time.

  void backgroundWorkerTx_DoWork(object sender, DoWorkEventArgs e)
  {
     if (this.InvokeRequired)
     {
        this.BeginInvoke(new EventHandler<DoWorkEventArgs>(backgroundWorkerTx_DoWork), sender, e);
        return;
     }
     //The text you wish to set should be supplied through the event arguments
     SetText((string)e.Argument);
  }

Answer 3

Try putting a lock over the section of the code that you believe you are having concurrency issues with.

lock(someObjectUsedOnlyForLocking)
{

}

Also, try using AppendText instead of manually concatenating the strings.

this.textBox1.AppendText(text);