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);
                    }
                }
            }
        }
    }
}

Are other threads trying to aquire the same lock? Could you be in a deadlock situation? — BradleyDotNET
– BradleyDotNET, Commented Dec 13, 2014 at 1:49
isn't that expected....if one thread gets the lock then all other threads would be waiting until the lock is released? — Steve
– Steve, Commented Dec 13, 2014 at 1:49
and I think the lock statement is wrong ..... he is trying to lock on a local string variable..not sure if that works — Steve
– Steve, Commented Dec 13, 2014 at 1:50
Yeah I agree that sounds a little bit odd Steve. I was also under the impression that Control.Invoke was designed to force the delegate code to be executed in the UI control loop thread. msdn.microsoft.com/en-us/library/… — Fuzz
– Fuzz, Commented Dec 13, 2014 at 1:53
@Fuzz now be careful there, although the Invoke method would make the delegate to be run on the Main thread, it is still possible that context switch would happen during the delegate execution which a second update could come thru before the first one finishes. — Steve
– Steve, Commented Dec 13, 2014 at 1:55

Peter Duniho · Accepted Answer · 2014-12-13 02:12:56Z

3

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.

answered Dec 13, 2014 at 2:12

Peter Duniho

71k7 gold badges113 silver badges149 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Piranha Over a year ago

I think what he wants to do is put a full text in a textbox, but adding slowly each char one-by-one to make a "slow-console" effect. That would explain the draw, the refresh and the sleep. And the reason he is trying to do that with a thread is, that he doesn't want the GUI to hang up while the effect is running.

Peter Duniho Over a year ago

Could be. But it's still the wrong way to do that. He could do that all in one thread, using the System.Windows.Forms.Timer class (which would be the right way). The code posted is wrong, regardless of what the goal is.

William Over a year ago

Ya, Piranha is correct. Sorry for bad presentation.

Collectives™ on Stack Overflow

Multithreading in C# using lock

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related