Thread synchronization with Monitor.Pulse NOT working as expected

Question

I have two methods, first() and second() and I assigned them to two threads.

Following are my second and first methods

     public void first()
    {
        lock (this)
        {
            Monitor.Wait(this);   

            Console.WriteLine("First");
            Monitor.Pulse(this);
        }
    }

    public void second()
    {
        lock (this)
        {
            //Monitor.Wait(this);
            Console.WriteLine("Second");
            Monitor.Pulse(this);
        }
    }

The problem is on console only "Second" is getting printed. Even though I have Monitor.Pulse() in second() to notify that the control should shift to first() but that aint happening.

I am following msdn following https://msdn.microsoft.com/en-us/library/aa645740(v=vs.71).aspx

Any help will be appreciated for info why the s thread is NOT shifting control to f thread with Monitor.Pulse() in second()

Follwinng is my Main()

        test t = new test();
        test1 t1 = new test1();
        Thread f = new Thread(new ThreadStart(t.first));
        Thread s = new Thread(new ThreadStart(t1.second));
        f.Start();
        s.Start();
        f.Join();
        s.Join();

score 4 · Accepted Answer · answered Sep 06 '16 at 08:15

4

The producer /consumer example works on one instance of cell. I think you should do the same. The reference to 'this' is different for your 'first' and 'second'.

answered Sep 06 '16 at 08:15

LoekD

11,402
17
27

2

Even once this is fixed, there's still a race here, that `second` assumes that `first` acquired the lock and entered `Wait` before `second` acquires the lock. (because `Pulse` without anyone `Wait`ing gets discarded) – Damien_The_Unbeliever Sep 06 '16 at 08:43
@Damien_The_Unbeliever But this answer is working correctly. – user786 Sep 06 '16 at 09:03
1

@Alex - it will - *some* of the time. It's a race to see which thread actually manages to execute the `lock(this)` line first and is not *guaranteed*. – Damien_The_Unbeliever Sep 06 '16 at 09:05
@Damien_The_Unbeliever How to fix it? – user786 Sep 06 '16 at 09:09
@Alex - that depends on what your actual problem/requirements are. Most of the fixes though are "ditch `Pulse`/`Wait` and use a more appropriate mechanism" - whether that be a different synchronization primitive or e.g. a thread-safe collection. – Damien_The_Unbeliever Sep 06 '16 at 09:13
@Damien_The_Unbeliever that means that producer/consumer example on msdn will also work some times while not other times. is that right? – user786 Sep 06 '16 at 09:15
@Alex - no. That code *conditionally* calls `Wait` and uses the `readerFlag` to ensure it's operating correctly. You code has an unconditional `Wait` and not enough logic to guarantee to the thread calling `Wait` that another thread will call `Pulse` in the *future*. – Damien_The_Unbeliever Sep 06 '16 at 09:38
@Damien_The_Unbeliever should I accept this answer OR r you going to give all these details in ur answer so I can accept it? – user786 Sep 06 '16 at 10:51
@Alex - you can accept it but be aware that just applying this fix has this race. If you want to avoid the race, we need more than just a toy example or more explanation of the work to be done by each thread, if you want to ask a new question about it. – Damien_The_Unbeliever Sep 06 '16 at 10:57

S. Jensen · Answer 2 · 2016-09-06T08:48:12.913

Since you initialize two instances of your Test-class (or you have two separate classes, Test and Test1), your usage of the this-keyword in the two methods refers to two separate objects.

Instead of locking on the context object, create a static object that's accessible to both methods, and lock on that:

public class Test
{
    static object _key = new object();

    public void First()
    {
        lock (_key)
        {
            Monitor.Wait(_key);
            Console.WriteLine("First");
            Monitor.Pulse(_key);
        }
    }

    public void Second()
    {
        lock (_key)
        {
            Console.WriteLine("Second");
            Monitor.Pulse(_key);
        }
    }
}

Put both First() and Second() in the same class, and create two instances of that class instead of one Test and one Test1, and you should see the results you expect, i.e. the output of the program will be

Second
First

EDIT: As Damien points out though, this is not guaranteed to work either, since it assumes that First() is the first to acquire the lock.

You'll only see that output if `First` is *in fact* the first method to successfully acquire the `lock`, which is by no means guaranteed when you spin off two threads to execute these methods. — Damien_The_Unbeliever, Sep 06 '16 at 08:47

Thread synchronization with Monitor.Pulse NOT working as expected

2 Answers2