-3

I have 3 threads that do different jobs and I need to collect the logs about them and write it to a text box. So I have:

public delegate void Notify(string message);

Each of the 3 classes of these threads has:

public event Data.Notify Notify;

(Data is just a static class where I keep general information for my app). In the method Work() that is in all of these 3 classes I use:

Notify?.Invoke("message");

In the message I have information if the job is started, if it was successful or not. In the class of the Form I got:

Data.workingThread.Notify += Thread_Notify;
Data.readingThread.Notify += Thread_Notify;
Data.writingThread.Notify += Thread_Notify;
Data.writingThread.Start();
Data.workingThread.Start();
Data.readingThread.Start();

Where Thread_Notify is:

private void Thread_Notify(string message)
{
    tbLogs.Text += message;
}

When I run the code, It throws System.InvalidOperationException as I try to access 'tbLogs' not from a thread where it was created. I tried async/await for this method and lock(tbLogs) but these didn't work for me

user13353974
  • 125
  • 7
  • You could `BeginInvoke()` there to `tbLogs.AppendText(message)`. Or replace the events with a [IProgress](https://learn.microsoft.com/en-us/dotnet/api/system.iprogress-1) delegate and `Report()` to the UI Thread. It's simpler to use Tasks instead of Threads in this case. How often will you add text to that control? – Jimi Oct 16 '20 at 22:01

2 Answers2

2

As I know, you can directly manage the text box only if you're in the main window thread. Since you event (Thread_Notify) can be called from other threads, you can't use it directly because if it runs in a thread different from the main window one, it will throw an Exception.

For let it work, you have to write the notify in this way:

private void Thread_Notify(string message)
{
   Dispatcher.Invoke(new Action(() =>
   {
      tbLogs.Text += message;
   }), DispatcherPriority.Background);
}
PiGi78
  • 415
  • 2
  • 6
  • `Dispatcher` isn't typically used in WinForms. – LarsTech Oct 16 '20 at 21:58
  • 1
    Sorry, I didn't see you're using WinForms. In that case, have you seen this post? https://stackoverflow.com/questions/519233/writing-to-a-textbox-from-another-thread – PiGi78 Oct 16 '20 at 22:02
1

Take a look at the Control.InvokeRequired property and the Invoke method.

Use them to dispatch the textbox changes on the UI thread.

Example:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        Random rnd = new Random();
        Action GetAction(string threadName) => () =>
        {
            for (int i = 0; i < 5; i++)
            {
                var tid = Thread.CurrentThread.ManagedThreadId;
                Thread_Notify($"{threadName} ({tid}): {i} ");
                int sleepMillis = 0;
                lock (rnd) sleepMillis = rnd.Next(0, 400);
                Thread.Sleep(sleepMillis);
            }
        };

        Task.WhenAll(
            Task.Run(GetAction("worker1")),
            Task.Run(GetAction("worker2")),
            Task.Run(GetAction("worker3"))
        ).ContinueWith(_ => Thread_Notify("Done."));
    }
    private void Thread_Notify(string message)
    {
        Action setText = delegate { tbLogs.Text += message + "\r\n"; };
        if (tbLogs.InvokeRequired)
        {
            tbLogs.Invoke(setText);
        }
        else
        {
            setText();
        }
    }
}

Another option is to use the System.Reactive.Linq library to do the dispatching for you:

  1. Create a subject.
  2. Call _subject.OnNext(message) in worker threads.
  3. Change the TextBox in a subscription.

Example:

using System.Reactive.Linq;
using System.Reactive.Subjects;
// ...
public partial class Form1 : Form
{
    private readonly Subject<string> _messages = new Subject<string>();
    public Form1()
    {
        InitializeComponent();
        Random rnd = new Random();
        Action GetAction(string threadName) => () =>
        {
            for (int i = 0; i < 5; i++)
            {
                var tid = Thread.CurrentThread.ManagedThreadId;
                _messages.OnNext($"{threadName} ({tid}): {i} ");
                int sleepMillis = 0;
                lock (rnd) sleepMillis = rnd.Next(0, 400);
                Thread.Sleep(sleepMillis);
            }
        };

        Task.WhenAll(
            Task.Run(GetAction("worker1")),
            Task.Run(GetAction("worker2")),
            Task.Run(GetAction("worker3"))
        ).ContinueWith(_ => _messages.OnNext("Done."));

        _messages
            .ObserveOn(this)
            .Subscribe(msg => tbLogs.AppendText(msg + "\r\n"));
    }
}
Gebb
  • 6,371
  • 3
  • 44
  • 56
  • There's no need to check `InvokeRequired`: it's clearly required, since the event is raised by a worker thread. You risk a deadlock calling `Invoke()` here. Almost certain if the Control or its container is disposed or otherwise *blocked* in the meanwhile (unless `Thread.Join()` is called at some point, then it's `101%` certain). `BeginInvoke()` is safer here. Concurrency is the only *problem*. – Jimi Oct 16 '20 at 22:59