CODEWITHSUNDEEP
c#multithreadingcounter
user5631123
user5631123

Reputation: 1

Thread and new counter

How to correct the code that each newly launched thread use the new counter. At the moment when you start a new thread hangs the old, instead of going on.

Thanks for help.

 private void button1_Click(object sender, EventArgs e)
    {

        thread[counter] = new Thread(goThread);
        thread[counter].Start();
        counter++; 
    }

    private void goThread()
    {
            kolejka[counter] = new PictureBox();
            kolejka[counter].Location = new Point(325, n - 150);
            kolejka[counter].Image = threading.Properties.Resources.car;
            kolejka[counter].Size = new Size(20, 37);

        this.Invoke((MethodInvoker)delegate
        {

            this.Controls.Add(kolejka[counter]);
        });


        for (int i = 0; i < 500; i++)
        {
            this.Invoke((MethodInvoker)delegate
            {
                kolejka[counter].Location = new Point(kolejka[counter].Location.X, kolejka[counter].Location.Y - 3);
                this.Refresh();
            });
            Thread.Sleep(50);
        }
    }

Upvotes: 0

Views: 172

Answers (4)

Thorsten Dittmar
Thorsten Dittmar

Reputation: 56697

The problem is that you're increasing the counter variable, but it is used within your threads. Don't do that. In your case it is very important to make information local to the thread, because you want each thread to work on "its own" counter. This can be achieved like this:

private class ThreadInfo
{
    public PictureBox Picture;
    public int Counter;
}

private void button1_Click(object sender, EventArgs e)
{
    kolejka[counter] = new PictureBox();
    kolejka[counter].Location = new Point(325, n - 150);
    kolejka[counter].Image = threading.Properties.Resources.car;
    kolejka[counter].Size = new Size(20, 37);
    this.Controls.Add(kolejka[counter]);

    ThreadInfo info = new ThreadInfo() {
        Picture = kolejka[counter],
        Counter = counter
    };

    thread[counter] = new Thread(goThread);
    thread[counter].Start(info);

    counter++; 
}

private void goThread(object state)
{
    ThreadInfo info = state as ThreadInfo;

    for (int i = 0; i < 500; i++)
    {
        this.Invoke((MethodInvoker)delegate
        {
            info.Picture.Location = new Point(info.Picture.Location.X, info.Picture.Location.Y - 3);
            this.Refresh();
        });
        Thread.Sleep(50);
    }
}

This does all the initialization stuff in your button event and passes in an instance of an info class. That info class takes all the information the thread needs, but so that it is local to the thread!

Upvotes: 3

Jeroen van Langen
Jeroen van Langen

Reputation: 22038

Follow up on Peter, you could create a copy at the beginning of the thread:

private void button1_Click(object sender, EventArgs e)
{
    ManualResetEvent threadStartedSignal = new ManualResetEvent(false);
    thread[counter] = new Thread(goThread);
    thread[counter].Start(threadStartedSignal);

    // wait for the thread to create a local reference.
    threadStartedSignal.WaitOne();
    counter++; 
}

private void goThread(object state)
{
    kolejka[counter] = new PictureBox();
    var myPictureBox = kolejka[counter];

    // signal the other thread, that the counter may be changed.
    ((ManualResetEvent)state).Set();

    myPictureBox .Location = new Point(325, n - 150);
    myPictureBox .Image = threading.Properties.Resources.car;
    myPictureBox .Size = new Size(20, 37);

    this.Invoke((MethodInvoker)delegate
    {
        this.Controls.Add(myPictureBox );
    });


    for (int i = 0; i < 500; i++)
    {
        this.Invoke((MethodInvoker)delegate
        {
            myPictureBox.Location = new Point(myPictureBox.Location.X, myPictureBox.Location.Y - 3);
            this.Refresh();
        });
        Thread.Sleep(50);
    }
}

Upvotes: 0

Jeroen van Langen
Jeroen van Langen

Reputation: 22038

There are several problems with your code.

  • You use a variable counter without locking in two threads.
  • Don't use arrays for this, because you don't have the counter value in control.
  • Don't create controls on other threads than the gui thread.

For this purpose, you don't need threads. The easiest way is using one timer.

PSEUDO:

List<Car> _myCars = new List<Car>();

private Form1()
{
    _timer = new Timer();
    _timer.Interval = 50;
    _timer.Tick += Timer_Tick;
}

private void Timer_Tick(object sender, EventArgs e)
{
    foreach(var car in _myCars.ToArray())
    {
        car.Location = new Point(car.Location.X, car.Location.Y - 3);
        if(car.Location.Y < 0)
            _myCars.Remove(car);
    }
}

private void button1_Click(object sender, EventArgs e)
{
    _myCars.Add(new Car());
}

Upvotes: 0

Peter
Peter

Reputation: 27944

Your old thread is not hanging. The problem is in your counter variable. It is shared by you threads. The old thread just continues on the kolejka[counter] of the new thread. I guess that is not what your want.

In the beginning of your goThread method you can do something like:

var item = kolejka[counter];

And then you can use item instead of kolejka[counter]. However this is not thread safe yet either, but a lot better then you have now.

Upvotes: 3

Related Questions