Unable to modify variables from separate thread

Question

So I'm making a C# app which has to continuously read and display the contents of a text file, while allowing the user to enter something into a text box and append it to the end of that very file.

I'm doing this by running my read method on a separate thread, however changing the variable which stores the display text-files contents is what's causing a problem. Initially I tried having a method which did this, however that's not working and gave a 'cross-thread-operation-not-valid' error. I then tried applying some code I found on MSDN, but now after updating the variable once the thread ended!

Please help.

partial class MainForm
{
    delegate void SetTextCallback(string text);
    public static string msg;
    public static string name;
    public void InitClient()
    {
        name = "public.txt";
        Console.WriteLine(name);
        if(!File.Exists(name))
        {
            File.Create(name);
            File.AppendAllText(name, "Welcome to " + name);
        }
        Thread Read = new Thread(new ThreadStart(this.Client));
        Read.Start();
        while(!Read.IsAlive);
    }
    public void WriteText()
    {
        File.AppendAllText(name, this.InputBox.Text);
        this.InputBox.Clear();
    }
    private void SetText(string text)
    {
        if (this.OutPut.InvokeRequired)
        {    
            SetTextCallback d = new SetTextCallback(SetText);
            this.Invoke(d, new object[] { text });
        }
        else
        {
            this.OutPut.Text = text;
        }
    }
    public void Client()
    {
        msg = File.ReadAllText(name);
        Console.WriteLine(msg);
        Thread.Sleep(300);
        this.SetText(msg);
    }
}

Why is the thread behaving like this. How can I modify my code so that the contents of the output box always equals that of the text file.

Any suggestions welcome.

`Client()` is a one-shot method. Of course it will terminate after 1 action, there is no loop. — H H, Jun 20 '15 at 08:37
That's a correct behavior, any control related data has to be changed in the UI thread context, you cannot do it on any other thread, here as I see following code in the SetText method would be an issue - this.OutPut.Text = text; you are trying to modify UI control on separate thread — Mrinal Kamboj, Jun 20 '15 at 08:40
@MrinalKamboj - that `else` part will only be hit after it was (recursively) Invoked. The SetText method here is correct. — H H, Jun 20 '15 at 08:41
Also Thread.Join is a better way to stop UI thread while another thread is executing, do not poll using While loop. Also why not use TPL, why still rely on thread class to do this work — Mrinal Kamboj, Jun 20 '15 at 08:42

score 1 · Accepted Answer · answered Jun 20 '15 at 08:40

1

You've got multiple problems here,

the use of the File is probably not thread-safe.
your method does not repeat
your are Sleep()ing on a Thread

You can solve all of them by ditching the Thread and use a simple Timer.

answered Jun 20 '15 at 08:40

H H

263,252
30
330
514

If i repeat the method, the application freezes, probably shouldve mentioned that. Using a time tho I can make it work. Thank you – Monacraft Jun 20 '15 at 08:44
Or a BackgroundWorker (see my answer) – havardhu Jun 20 '15 at 08:46

havardhu · Answer 2 · 2015-06-20T09:07:19.583

Try using a background worker instead of creating a new thread. The background worker will run its content in a seperate thread, and allows you to report 'progress' while its working. This progress report will always be run on the UI-thread (or the thread which started the background worker).

It also has an event which is called when the background worker is finished. This is also run on the UI thread.

This example should get you started.

Update: Added some very basic error handling as suggested

The idea is to use the UserData (2nd argument) of ReportProgress to do updates on the UI thread whenever you need to. In this case it is a string, but this can be any object.

Furthermore, you can use the Result of the DoWorkEventArgs to produce a final result from the background work. In this case, I return any exception which was thrown, or null otherwise, but you can return whatever you want here as well.

It is, as Henk mentioned in his comment, very important to handle errors that occur inside the DoWork callback, because exceptions etc which occurs here will be swallowed and the worker will complete as if nothing bad happened.

private BackgroundWorker _backgroundWorker;

public Form1()
{
    InitializeComponent();

    _backgroundWorker = new BackgroundWorker();
    _backgroundWorker.WorkerReportsProgress = true;
    _backgroundWorker.WorkerSupportsCancellation = true;
    // This is the background thread
    _backgroundWorker.DoWork += BackgroundWorkerOnDoWork;

    // Called when you report progress
    _backgroundWorker.ProgressChanged += BackgroundWorkerOnProgressChanged;

    // Called when the worker is done
    _backgroundWorker.RunWorkerCompleted += BackgroundWorkerOnRunWorkerCompleted;
}

private void BackgroundWorkerOnRunWorkerCompleted(object sender, RunWorkerCompletedEventArgs runWorkerCompletedEventArgs)
{
    if (runWorkerCompletedEventArgs.Result != null)
    {
        // Handle error or throw it
        throw runWorkerCompletedEventArgs.Result as Exception;
    }

    textBox1.Text = "Worker completed";
}

private void BackgroundWorkerOnProgressChanged(object sender, ProgressChangedEventArgs progressChangedEventArgs)
{
    textBox1.Text = progressChangedEventArgs.UserState as string;
}

private void BackgroundWorkerOnDoWork(object sender, DoWorkEventArgs doWorkEventArgs)
{
    try
    {
        for (int i = 0; i < 100 && !_backgroundWorker.CancellationPending; i++)
        {
            _backgroundWorker.ReportProgress(0, i + " cycles");
            Thread.Sleep(100);
        }
    }
    catch (Exception ex)
    {
        doWorkEventArgs.Result = ex;
    }

}

private void startButton_Click(object sender, EventArgs e)
{
    if (!_backgroundWorker.IsBusy)
        _backgroundWorker.RunWorkerAsync();
}

private void cancelButton_Click(object sender, EventArgs e)
{
    if(_backgroundWorker.IsBusy)
        _backgroundWorker.CancelAsync();
}

That still `Sleep()`s on a Thread, but that's a minor issue. Leaving out all error handling is more serious, this leads to "it doesn't work but there is no error" . — H H, Jun 20 '15 at 08:50
@HenkHolterman This is just an example of how the background worker works, the sleep there is irrelevant. Good point re: error handling though, will update — havardhu, Jun 20 '15 at 08:55
The error handling is already part of the Bgw, see [my answer here](http://stackoverflow.com/a/30707076/60761) for instance. — H H, Jun 20 '15 at 09:54

Unable to modify variables from separate thread

2 Answers2