Multithreading in C# using lock

Question

sorry for the bad presentation before. I edited the code and now it gives me the required result. So now when a thread writes in the richtextbox, the other threads do not freeze. I don't know whey I don't need to refresh the richtextbox her after adding a character! However, I'm still confused. Sometimes when I don't use Invoke method with any control I got an error, but now, as u can see in

panel.BackColor = Color.Red;

The compiler did not complain. Why?

    namespace ThreadGUI
{

public partial class Form1 : Form

    {
        private Size s = new Size(50, 50);
        Point p; 

        public Form1()
        {
            InitializeComponent();
        }

        private void Form1_MouseClick(object sender, MouseEventArgs e)
        {
            p = new Point(e.X, e.Y);
            Thread th = new Thread(new ThreadStart(DoWork));
            th.Start();
        }

        public void DoWork()
        {
            Panel panel = new Panel();
            Form1 frm = new Form1();
            Point p1 = new Point(p.X - 25, p.Y - 25);
            panel.Location = p1;
            panel.Size = s;
            panel.BackColor = Color.BlueViolet;
            this.Invoke(new MethodInvoker(delegate { this.Controls.Add(panel); }));

            Random ri = new Random();
            while (true)
            {
                panel.BackColor = Color.BlueViolet;
                int ti = ri.Next(500);
                while (ti > 0)
                {
                    int xi = ri.Next(2) * 10 - 5;
                    int yi = ri.Next(2) * 10 - 5;
                    Thread.Sleep(10);
                    p1.X += xi;
                    p1.Y += yi;
                    panel.Invoke(new MethodInvoker(delegate { panel.Location = p1; }));
                    ti--;
                }
                panel.BackColor = Color.Red;
                lock ("jkj")
                {
                    panel.BackColor = Color.Green;
                    string str = "I am a thread";
                    foreach (char c in str.ToCharArray())
                    {
                        richTextBox1.Invoke(new MethodInvoker(delegate { richTextBox1.AppendText(c.ToString()); }));
                        Thread.Sleep(100);
                    }
                }
            }
        }
    }
}

Answer 1

Sorry to be the bearer of bad news, and I mean no offense, but the posted code is all kinds of wrong. :(

First, I don't know what draw() does, but if it has anything to do with the UI, it's probably wrong to put it there. It probably should be executed on the UI thread along with the update of the text.

Second, what are those Sleep() method calls doing there? Whatever you're trying to accomplish with them, there is a better way. The second call seems particularly arbitrary and harmful.

Third, it is a bad idea to ever call Control.Invoke() while holding any kind of lock. There is no obvious deadlock here, but that may be just because you haven't posted a good code example and we can't see the part that's causing the deadlock. The Invoke() call itself is essentially a lock as well, as no other code will run on the UI thread until that method returns, and so that along with the first lock set you up for a possible deadlock (which is what sounds like is happening to you).

Fourth, don't call Control.Refresh(). It's not needed. Updating the text in the control will cause the control to become "invalidated", which will automatically result in the control being redrawn at an appropriate time. You don't need to hurry things along here, and doing so may wind up interfering with other things in your code.

Finally, I don't understand the stated requirement: that you are using the lock to prevent more than one thread from changing the text. That's exactly what Invoke() will do! Since all invoked delegates are executed on the UI thread, only one of them can execute at a time. So using Invoke() already necessarily prevents more than one thread from changing the text at a time.

In other words, if the only reason you added the lock statement was to accomplish that stated goal, you don't need it. The goal is already accomplished without it.