3

To keep my question simple lets says that I use a thread to update on the fly a replacement in a string. In my real code I need a thread. I know I can avoid it in this simple example.

So, my software has two fields. The user select a file, write a (sort of) regex and see the result of the modification at the same time he types his sentence. I start the thread when the user select a file (see listViewFiles_SelectionChanged method). The work of my thread is in DoWork method.

public void DoWork()
    {
        while (true)
        {
            FileData fileData = _selectedFile;
            if (fileData != null)
            {
                string name = fileData.FileName;
                string searchRegEx = GenerateRegex(_searchTextBox.Text);
                string replacement = _replaceTextBox.Text;
                name = Regex.Replace(name, searchRegEx, replacement);
                /*
                foreach (var action in _actionCollection)
                {
                    name = action.Rename(name);
                }*/

                _searchSample.Content = fileData.FileName;
                _replaceSample.Content = name;
            }
            Thread.Sleep(1000);
        }
    }

    private void listViewFiles_SelectionChanged(object sender, SelectionChangedEventArgs e)
    {
        _selectedFile = listViewFiles.SelectedItem as FileData;
        _thread.Start();
    }

When my thread do his work I get an exception on the line string searchRegEx = GenerateRegex(_searchTextBox.Text); : The calling thread cannot access this object because a different thread owns it. I read a lot about this exception but I don't understand it.

To fix it I surround my code with a Dispatcher. I don't understand the mecanism but it works. I have no idea if it is correct or performant but it works.

 public void DoWork()
    {
        while (true)
        {
            FileData fileData = _selectedFile;
            if (fileData != null)
            {
                //use Window.Dispatcher
                this.Dispatcher.Invoke(System.Windows.Threading.DispatcherPriority.Normal,
                    new Action(delegate()
                    {
                        string name = fileData.FileName;
                        string searchRegEx = GenerateRegex(_searchTextBox.Text);
                        string replacement = _replaceTextBox.Text;
                        name = Regex.Replace(name, searchRegEx, replacement);
                        /*
                        foreach (var action in _actionCollection)
                        {
                            name = action.Rename(name);
                        }*/

                        _searchSample.Content = fileData.FileName;
                        _replaceSample.Content = name;
                    }));
            }
            Thread.Sleep(1000);
        }
    }

    private void listViewFiles_SelectionChanged(object sender, SelectionChangedEventArgs e)
    {
        _selectedFile = listViewFiles.SelectedItem as FileData;
        _thread.Start();
    }

I would like to know if this code is correct and prfomant. You see the foreach instruction in comment. My code should do a lot of work and I don't know if doing this in a delagate is the best way do do this. Utility and good practice of the Dispatcher ?

Bastien Vandamme
  • 17,659
  • 30
  • 118
  • 200
  • 2
    Why you don't use mvvm at all ? Dispatcher.Invoke just invokes delegate synchronously in UI thread, so this code can cause ui freezes. You got error because trying to access _searchTextBox element in different thread, so you just need to separate ui updating & work in code. – trimeyko May 18 '12 at 14:29
  • @trimeyko Because I am not comfortable with MVVM. But you're right, I should use it. – Bastien Vandamme May 18 '12 at 14:33
  • Did you consider to use a BlockingQueue as quick fix? From UI you push _replaceTextBox.Text and in the other thread you perform the work. – Adriano Repetti May 18 '12 at 14:48
  • @trimeyko, I don't understand why MVVM would solve the problem. Threading is even harder with binding, btw, because you have a hard time diagnosing the problem. – Bruno Brant May 18 '12 at 15:42

4 Answers4

5

I think there are many questions in your single post, and I'll try to address them all:

Accessing Visual Controls From Other Threads

Both Winforms and WPF were build based on the fact that only a single thread can change the object state, and that thread is, of course, the same that created the object.

You can imagine why this is important: controls are objects that knows how to "render" or "draw" themselves. While drawing, re-sizing, moving/being dragged, properties of the object cannot be changed from "outside" of the control itself. Of course, the main thread is already busy doing the transformations I mentioned, so its guaranteed that "user" code running on the main thread won't change it. However, another thread running in parallel might do exactly that. Imagine, for example, that the main thread is rendering a text from a TextBox and half the word was written when a second thread changes the text. This would cause problems for the calculation of the width of the text for instance.

Using Dispatcher

What the thread dispatcher does is * marshal* your code to the main thread. You see, most visual frameworks, WinForms and WPF included, are based on a "application loop". This means that your application in running inside a while(true){} block. Your code (for instance, listViewFiles_SelectionChanged) is called from this loop when its needed. The object that manages this calls is the Dispatcher. It has a queue of stuff to run, and it decides that to run next. When the code the dispatcher called is run, nothing else happens on the visual part of the application -- after all, that is what the thread is doing, right? So it can't process user input, re render the screen, etc.

The Dispatcher provides you with an interface that can be called from another thread which posts a new method to be called by the dispatcher, by inserting it on the queue. It's not immediately executed, as you can understand: you are on the second thread calling the main, and the main might be busy either rendering the screen, processing input or even running your code such as listViewFiles_SelectionChanged. Once that loop iteration ends, Dispatcher will check the queue. Depending upon the priority of your method, it might be the next one to be executed or even wait a few more iterations.

For this reason, if you stick everything you are doing in the second thread on the dispatcher method, you are effectively asking the framework to run your second thread code on the main thread. And, since your thread code runs forever, the main thread will be forever busy runnning that code (DoWork) and will no longer be able to do anything else.

Dispatcher Good Practices

So, in conclusion of the above items, when you marshall code to the dispatcher, the main thread gets busy doing it. And while busy, your application becomes unresponsive. Since we want a responsive app all the time, we must do as little as necessary on the main thread, that is, on whatever we ask the Dispatcher to marshal.

That being the case, what you must do is only marshal lines that access Controls. Even if that means many calls to the Dispatcher -- you'll pay a price for that calls, of course, but it's better than stuck the Main Thread with your code longer than necessary.

Using a Dispatcher With A Single-Thread

The Dispatcher can also help you even if you don't have a second thread. If you need to perform long calculations, you can use a set of flags or a enum to keep track of the state. In the method, you call the dispatcher passing your own method (with a low priority) and them returning (so you break your calculation in parts).

Bruno Brant
  • 8,226
  • 7
  • 45
  • 90
1

The problem is, that your code is accessing _searchTextBox.Text and __replaceTextBox.Text. Your solution with dispatcher is working, but actually does not solve anything. The code will be executed in UI thread, but not immediatelly after ListView selection change.

To make it work go back to your first version without dispatcher, but pass SearchText and ReplaceText as thread start arguments. here is some pseudo-code:

var searchText = _searchTextBox.Text;
var replaceText = _replaceTextBox.Text
Thread.Start(() => DoWork(searchText, replaceText));
Ondra
  • 1,619
  • 13
  • 27
0

You can access your controls only in the thread in which the were created. In most cases it is the UI thread. This was the case with Windows Forms and it is the case with WPF. Calling Dispatch just blocks your thread and runs the rest in the UI thread, which takes away most of the benefits of offloading work to other threads and of course prevents the UI thread from doing other work. The solution for this, as somebody mentioned above, is to separate your UI code from the background worker code. In this case it is simple, because you are only using input values from the interface which you can get from a Queue to which you add items in the UI thread. Neeed to synchtronize access to the queue to avoid racing conditions (lock(queue.Synch)). This is a classic case of the Produce-Consumer design pattern.

j0aqu1n
  • 1,013
  • 7
  • 14
0

So, if we just look at your code I'm suggest to wrap ui update/get code to invokes only, because as I said currently you just invoke your code in UI thread and your additional thread useless, because it's just invoke & sleep.

Here is an example :

 private Thread _thread;

    public MainWindow()
    {
        InitializeComponent();

        _thread = new Thread(DoWork);

        _thread.Start();
    }

    private void DoWork()
    {
        while (true)
        {
            var str = (string)Dispatcher.Invoke(new Func<object>(() => NotifyLabel.Content));

            str += "a";

            Dispatcher.Invoke(new Action(() => NotifyLabel.Content = str));

            Thread.Sleep(500);
        }
    }

I'm suggest you to read invoke info & there was interesting question about dispatcher/threading

P.S.: I'm not reviewing your multithread code in general, because it can be greatly improved if you use mvvm/other patterns here

Community
  • 1
  • 1
trimeyko
  • 129
  • 6