Cross-thread operation not valid: Control 'textbox' accessed from a thread other than the thread it was created on

Question

I need some help. I started c# and not very familiar with event handling and threading yet. As a beginner and as time and exposure progresses, I would like to learn more on these advanced topics and improved and hope all of you here can help me.

I ran onto this problem of "Cross-thread operation not valid: Control 'textbox control called stackStatus' accessed from a thread other than the thread it was created on". I have tried to troubleshoot this whole day but simply no avail. I am stuck. :-( The program hits an exception and cannot continue to execute smoothly.

I have read the following threads and tried a few things but I guess I am still missing something. Appreciate if someone can help me out here. Thanks.

Cross-thread operation not valid: Control accessed from a thread other than the thread it was created on

Cross-thread operation not valid: Control 'textBox1' accessed from a thread other than the thread it was created on

Here's are most of the portion of the code:

  private void createCloud_Click(object sender, EventArgs e)
    {
        CreateCloud(); //start creation method
        stackStatus.Text = "Creating stack..."; //updates the cloud status textbox
        stackStatus.Refresh();
        Cursor.Current = Cursors.WaitCursor; //change the cursor to wait state
        Start_Describestack(); //call describe method to find out the status of cloud creation progress

        Task.Delay(12000); // wait 12s in case not ready

        Start_Describestack(); // call again describe method to find out the cloud creation progress status
        Cursor.Current = Cursors.Default; //put cursor on wait
        describeevents(); // call method to get all cloud creation event data and publish on the datagridview

    }



    private void Start_Describestack()
    {
        //method making use of timer to call 

        _timer = new System.Timers.Timer(15000);
        _timer.Elapsed += new ElapsedEventHandler(describeStack);
        _timer.Enabled = true;


    }

    delegate void describeStackCallBack(object sender, ElapsedEventArgs e);

    private void describeStack(object sender, ElapsedEventArgs e)
    {
        //this method makes api calls through cloudclient to describe the stack
        //this is where the "Cross-thread operation not valid: Control 'stackStatus' accessed from a thread other than the thread it was created on"


        var client = new cloudclient();
        var request2 = new StacksRequest();

        request2.Cloudstackname = stackid;

        try
        {
            var response = client.DescribeCloudStacks(request2);

            foreach (var stack in response.Stacks)
            {

            //something is wrong here but I do not know how to fix it. Please help
                if (this.stackStatus.InvokeRequired)
                {
                    describeStackCallBack d = new describeStackCallBack(describeStack);
                    this.Invoke(d, new object[] { sender, e });
                    stackStatus.Refresh();
                    describevents();
                }
                else
                {
                    stackStatus.Text = stack.StackStatus;
                    stackStatus.Refresh();
                    describeevents();
                }
            }
        }
        catch (Exception)
        {

            if (this.stackStatus.InvokeRequired)
            {
                describeStackCallBack d = new describeStackCallBack(describeStack);
                this.Invoke(d, new object[] { sender, e });

                stackStatus.Text = "Stack not found/Deleted";
            }
            else
            { stackStatus.Text = "Stack not found/Deleted"; }
        }

     describeevents();
    }

    private void describeevents()
    {


        var newclient = new cloudclient();
        var request3 = new eventrequest();

        request3.Cloudstackname = stackid;


            try
            {
                var response = newclient.eventstack(request3);
                dataGridView3.Rows.Clear();

                foreach (var events in response.sevents)
                {
                    dataGridView3.Rows.Add(events.Timestamp, events.ResourceStatus, events.ResourceType);
                }
            }
            catch (Exception)
            {
                dataGridView3.Rows.Clear();
                MessageBox.Show("Stack not ready!");
            }

        dataGridView3.Refresh();

    }

Answer 1

Rather than doing :

stackStatus.Text = "some text";

Try :

stackStatus.Invoke((Action)delegate
{
    stackStatus.Text = "some text";
});

Note that GUI element assignment outside the thread where they are declared is deprecated because the controls may no longer be available at any time.

Answer 2

There are two issues in your approach, which conspire to prevent your attempt to imitate the solution to the exception from working:

1. You have failed to note that the proposed solution calls itself, and in so doing, causes the foreach to be restarted for each time it's invoked from the worker thread.
2. You are following Microsoft canonical implementation of cross-thread-friendly Invoke()-based code, which IMHO is lame.

It is my opinion that there is no point in ever checking InvokeRequired. The standard pattern always involves situations where on the first entry, you know you will require Invoke(), and even if you didn't, there's no real harm in calling Invoke() when it's not necessary.

Instead, you should always keep separate the code that should run in the UI thread, and the code that does not. Then, in the code that does not, always use Invoke() to execute the code that does.

For example:

private void Start_Describestack()
{
    //method making use of timer to call 
    _timer = new System.Timers.Timer(15000);
    _timer.Elapsed += new ElapsedEventHandler(_timer_Elapsed);
    _timer.Enabled = true;
}

private void _timer_Elapsed(object sender, ElapsedEventArgs e)
{
    Invoke((MethodInvoker)describeStack);
}

private void describeStack()
{
    var client = new cloudclient();
    var request2 = new StacksRequest();

    request2.Cloudstackname = stackid;

    try
    {
        var response = client.DescribeCloudStacks(request2);

        foreach (var stack in response.Stacks)
        {
            stackStatus.Text = stack.StackStatus;
            stackStatus.Refresh();
            describeevents();
        }
    }
    catch (Exception)
    {
        stackStatus.Text = "Stack not found/Deleted";
    }

    describeevents();
}

That said, an improvement on the above would be to use System.Windows.Forms.Timer instead of System.Timers.Timer. The latter raises the Elapsed event on a worker thread, but the former raises its event on the UI thread, right where you want it. No Invoke() required at all.

You have at least one other problem with your code as well:

private void createCloud_Click(object sender, EventArgs e)
{
    CreateCloud(); //start creation method
    stackStatus.Text = "Creating stack..."; //updates the cloud status textbox
    stackStatus.Refresh();
    Cursor.Current = Cursors.WaitCursor; //change the cursor to wait state
    Start_Describestack(); //call describe method to find out the status of cloud creation progress

    Task.Delay(12000); // wait 12s in case not ready

    Start_Describestack(); // call again describe method to find out the cloud creation progress status
    Cursor.Current = Cursors.Default; //put cursor on wait
    describeevents(); // call method to get all cloud creation event data and publish on the datagridview
}

In the above, the call to Task.Delay(12000); accomplishes nothing. The Task.Delay() method doesn't actually block the current thread. Instead, it returns an awaitable task object. The code in which it appears only is delayed if you wait on the returned object.

It's also questionable to call Start_Describestack() twice, because this method doesn't do anything except start the timer. Calling it twice means now you have two timers running.

Finally, you should also not have all those calls to Refresh() in your code. Correctly written Windows Forms code will not need anything like that. Updates to control properties will cause control invalidation automatically, and the control will update as needed at its next opportunity, which as long as the code is written correctly, will be soon enough for the user to not notice any significant delay.

Now, putting all of the above together, it seems to me that you should avoid using the timer altogether. There is still the potential problem that your call to DescribeCloudStacks() is a lengthy one, and could cause the UI to momentarily appear "stuck", which obviously isn't a desirable thing. In addition, the timer-based code, whether you require Invoke() or not, can be harder to understand, especially for someone new to asynchronous programming and threading.

Using the async/await feature, you can write the code in a conventional, procedural way, while still ensuring that the UI remains responsive, and that the UI-related code is always executed in the UI thread where it belongs. That might look something like this:

private async void createCloud_Click(object sender, EventArgs e)
{
    CreateCloud(); //start creation method
    stackStatus.Text = "Creating stack..."; //updates the cloud status textbox
    Cursor.Current = Cursors.WaitCursor; //change the cursor to wait state
    await describeStack(); //call describe method to find out the status of cloud creation progress

    await Task.Delay(12000); // wait 12s in case not ready

    await describeStack(); // call again describe method to find out the cloud creation progress status
    Cursor.Current = Cursors.Default; //put cursor on wait
    describeevents(); // call method to get all cloud creation event data and publish on the datagridview
}

private async Task describeStack()
{
    var client = new cloudclient();
    var request2 = new StacksRequest();

    request2.Cloudstackname = stackid;

    try
    {
        var response = await Task.Run(() => client.DescribeCloudStacks(request2));

        foreach (var stack in response.Stacks)
        {
            stackStatus.Text = stack.StackStatus;
            describeevents();
        }
    }
    catch (Exception)
    {
        stackStatus.Text = "Stack not found/Deleted";
    }

    describeevents();
}

The above executes most of the describeStacks() method in the UI thread. The exception would be the DescribeCloudStacks() method call, which is run as a worker task. While it's running, the UI thread is free to operate normally. Execution of the describeStacks() method is temporarily put "on hold" (without blocking the UI thread) while the worker task runs, and then is resumed when it completes.

It's not clear from your original example whether you really wanted a repeating timer or not. The above doesn't use any loops; it calls the describeStack() method only twice, with a 12-second delay in between. But if you want a loop, you can do that as well. Just use the await Task.Delay() for the delay and await describeStack() for the operation, and put that in a loop as you like.

Answer 3

I don't see where the stackStatus object is created so I'm just guessing that you are creating it through a contructor for the class containing describeStack() and then you are registering an event handler for the click. I think what is happening is the event handler is being run on a different thread from the one in which the instance was created so you might have to change how you create the stackStatus object. That error is likely happening because whatever type the stackStatus was created from is known to not be reentrant so when the runtime detects access between threads it raises an exception so you are aware and can either prevent or recover from race-conditions or deadlocks.