Creating objects in a main thread and working with them in another thread. How to do that?

Question

Right Now, it outputs 00, instead of 54. I'm learning with threads and I stuck here, I don't know what to do now

namespace WindowsFormsApplication2
{
    public partial class Form1 : Form
    {
        Point[] array = new Point[20];
        public Form1()
        {
            for (int i = 0; i < 20; i++)
            {
                array[i] = new Point(); // I'm creating objects here
            }
            InitializeComponent();
        }

        void function1()
        {
            array[0].x = 5;
            array[0].y = 4;
        }

        void function2()
        {
            label1.Text = array[0].ToString();
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            Thread thread1 = new Thread(function1);
            thread1.Start();
            Thread.Sleep(500);
            function2();
        }
    }

    class Point
    {
        public int x;
        public int y;
        public override string ToString()
        {
            return x.ToString() + y.ToString();
        }
    }  
}

when Im doing it in that way (without another thread)

private void Form1_Load(object sender, EventArgs e)
{
    function1();
    function2();
}

It works fine, output is 54 thanks

There a good ( and free ) beginners ebook here: [Albahari](http://www.albahari.com/threading/) — Nick Butler, Apr 19 '12 at 10:25
Your code is ok. Are you sure you have attached your event handler `Form1_Load` to the `Form.Load` event? Also, you should add a breakpoint in `function1` and check if your method is called. — Paolo Moretti, Apr 19 '12 at 10:35
What happens if you try conventional debugging? If you put breakpoints on 'thread1.Start()', 'array[0].x = 5;' and 'function2();', what happens? — Martin James, Apr 19 '12 at 10:38
your code should print 54 as expected. there is something wrong — Fer, Apr 19 '12 at 10:55
There is a missing ';' after the sleep call - will not compile. — Martin James, Apr 19 '12 at 11:03
@MartinJames: It's probably a typo. I went ahead and fixed it. — Brian Gideon, Apr 19 '12 at 13:57

score 2 · Answer 1 · answered Apr 19 '12 at 10:28

2

What you are doing does not make much sense. You are starting a thread, then sleeping for 500ms and then calling the function that uses the results of that thread.

This means that your two actions are sequentially dependent and cannot execute in parallel. Using threads here is meaningless.

And the reason you are not seeing the changes is probably because thread1 does not have enough time to start and execute its code before the sleep finishes and function2 is called. Try to use thread1.Join() instead, but like I said, this makes no sense at all!

answered Apr 19 '12 at 10:28

Tudor

61,523
12
102
142

500ms? Surely it should be done by then! I'm 100% in agreement that the long sleep() is unsafe and there are better mechanisms available to signal completion of a task, but even so, the OP code should 'work'. – Martin James Apr 19 '12 at 10:36
@Martin James: It seems suspicious to me too. However, I would not ponder on this subject longer because the MRE solution above seems to work. Also, as I said, the OP's scenario makes the use of threads useless, since he's starting a thread and joining it immediately. – Tudor Apr 19 '12 at 11:36
thanks for the reply, however I think MRE.Set(); is prolly better method since I do have an infinite loop in my method, i can place it whenever I want to. I also know that this program doesn't make any sense but still, it was only example. I though It may be better to show smth like that than pasting my whole real code :) :P – Patryk Apr 19 '12 at 12:20

score 2 · Accepted Answer · answered Apr 19 '12 at 11:07

2

Actually your program is working on my computer as expected. So why it's not working on your computer (or is working only sometimes)? This is because there is race condition in your program. This means that your program assumes that the second thread (that executes function1) will assign new values to array in less than 500ms. Never make any ASSUMPTION about how long it takes for the other thread to complete it's task. Operating system is free to choose when, how often and how long individual threads executes and when it suspends them. Instead, use synchronization primitives (such as ManualResetEvent) to MAKE SURE that the second thread has done it's task.

This code shows, how you can change your program do thread synchronization properly:

    ManualResetEvent MRE = new ManualResetEvent(false);//Add this field to Form1

    void function1()
    {
        array[0].x = 5;
        array[0].y = 4;
        MRE.Set();//This notifies first thread, that values are set.
    }

    private void Form1_Load(object sender, EventArgs e)
    {
        Thread thread1 = new Thread(function1);
        thread1.Start();
        MRE.WaitOne();//This makes this thread wait until the second thread calls MRE.Set()
        function2();
    }

answered Apr 19 '12 at 11:07

Ňuf

6,027
2
23
26

What you say is correct, but it's almost beyond belief that the OP code would not 'work' on any system that is not overloaded. 500ms is nearly forever. – Martin James Apr 19 '12 at 11:13
Why use an MRE when `Thread.Join` exists exactly for this scenario? – Tudor Apr 19 '12 at 11:34
@Martin James: 500ms seems to be enough, but as you can see, sometimes it is not. Although this is probabably not the case, but I believe that because of lack of any memory barrier in original program, compiler could reorder Sleep() and function2() calls and thus read array[0] without waiting. WaitOne() calls memory barrier implicitly which prevents such reordering. – Ňuf Apr 19 '12 at 12:43
@Tudor: Thread.Join() would really be good choice in this simple example. My MRE version is more universal and shows how to use synchronisation primitive, which doesn't require second thread to terminate and is more common in non-trivial scenarios. – Ňuf Apr 19 '12 at 12:43
This fixes the race condition, but introduces another problem. `WaitOne` is a blocking call that does not pump messages so this will hang up the UI thread. – Brian Gideon Apr 19 '12 at 13:52
I really, really hope that the compiler does not reorder my system calls. If it does, it is unuseable for anything except trivial apps. – Martin James Apr 19 '12 at 15:46
@Martin James: Compiler cannot reorder system calls, because it knows nothing about potential side-effects of these calls to unmanaged code. But managed code can be freely reordered within critical points (defined as volatile read/write, lock, thread start, thread end) as long as data dependencies are preserved. There are no critical points between Sleep() and function2() calls, so i believe reordering is allowed (at least i didn't find paragraph in C# spec, that would forbid this reordering - tell me if you can find the opposite). – Ňuf Apr 19 '12 at 16:07
@Ňuf: You're definitely thinking about the reordering problem the right way. Fortunately, `Thread.Sleep` generates a memory barrier. At least, it is on my [speculated list](http://stackoverflow.com/a/6932271/158779) at the momement because my own personal experimentation seems to indicate as much. So, in this particular case it is a moot point, but your thinking is solid generally speaking. – Brian Gideon Apr 20 '12 at 01:06
If compilers carry on like this, we'll all be back to writing single-threaded 'hourglass' apps because optimizations make synchronization and/or signaling too dangerous. – Martin James Apr 20 '12 at 09:01
Yeah, well, unfortunately they do. And it is pretty [easy to replicate](http://stackoverflow.com/a/3556877/158779). – Brian Gideon Apr 20 '12 at 14:07
Can we solve this man-to-man by putting Intel chip designers and M$ compiler developers onto the same Ice Hockey rink so they can sort out their differences in design-goals? I'm just a developer, why do I have to put up with this? It's not as if software development is too easy as it is! – Martin James Apr 21 '12 at 00:47

score 1 · Answer 3 · answered Apr 19 '12 at 13:50

If function2 is dependent upon the completion of function1 then you need to make sure function1 really does complete before function2 starts. Using Thread.Sleep for that job will not always work because of the unpredictable nature of how threads are scheduled and executed.

Another problem with Thread.Sleep is that it is blocking the UI thread. The number one rule when using multithreading in UI based application is to never block the UI thread. This applies to any kind of blocking call including Thread.Sleep, ManualResetEvent, Thread.Join, etc.

The way I would approach this problem is to use the Task class with the ContinueWith method. This will wait for function1 to complete before calling function2 without blocking the UI thread.

private void Form1_Load(object sender, EventArgs e)
{
  TaskScheduler ui = TaskScheduler.FromCurrentSynchronizationContext();

  Task.Factory.StartNew(() =>
  {
    function1();
  }).ContinueWith((task =>
  {
    function2();
  }, ui);
}

Martin James · Answer 4 · 2012-04-19T11:06:33.813

You code, as supplied, will not compile because of the missing ';' afer the Sleep(500) call.

Whwn the syntax is fixed, it 'works' on my box.

You have no secure way of signalling that it has finished. You are learning so, first lesson - don't wait for threads with sleep() calls.

A BackgroundWorker component would be easier to use - you don't have to worry about explicit signalling to get a result back.

If you want to stick with the Thread instance, look at autoResetEvent class or Thread.Join().

enter image description here

Creating objects in a main thread and working with them in another thread. How to do that?

4 Answers4