C# 3.0 - Application of Monitor -Suggestions

Question

In order to understand the Monitor I have implemented the following code.But I am not sure whether the code is Thread Safe.

namespace MonitorExample
{
    public delegate void WaterLevelInformer(object sender,WaterLevelArgs e);

    class WaterLevelListener
    {
        //listener will print information
        // when WaterTank is reaching its empty level or full
        public void ShowResult(object sender, WaterLevelArgs e)
        {
            Console.WriteLine("Water Level is :{0}", e.Level);
        }
    }

    class WaterTank
    {
        //starting level is empty
        static int level=0;
        //capacity of the WaterTank is 2000 liters
        const int capacity = 2000;
        private WaterLevelListener lst = new WaterLevelListener();

        public event WaterLevelInformer levelHandler;

        public WaterTank(WaterLevelListener lstn)
        {
            this.lst = lstn;
            this.levelHandler +=new WaterLevelInformer(lst.ShowResult);
        }

        public void FillWater()
        {
            lock (this)
            {
                if (level >= capacity)
                {
                    Monitor.Wait(this);
                }
                Console.WriteLine("....WaterTank is gettig filled...");
                for (int i = 100; i <= 2000; i+=100)
                {
                    Console.WriteLine("Current Water Level {0}", i);
                    level = i;
                    Thread.Sleep(1000);
                    if (i == 1700)
                    {
                        WaterLevelInformation(level);
                        Thread.Sleep(1000);
                    }
                }

                Monitor.Pulse(this);
            }
        }

        public void ConsumeWater()
        {
            lock (this)
            {
                if (level<=0)
                {
                    Monitor.Wait(this);
                }
                Console.WriteLine("...Water is being consumed....");
                for (int i =2000; i >= 0; i -= 100)
                {
                    Console.WriteLine("Current Water Level {0}", i);
                    Thread.Sleep(1000);
                    level = i;
                    if (i == 100)
                    {
                        WaterLevelInformation(i);
                        Thread.Sleep(1000);
                    }
                }

                Monitor.Pulse(this);
            }
        }

        //WaterLevelInformation is used to raise the event
        // When WaterTank reaching its empty level
        //or WaterTank is full 
        public void WaterLevelInformation(int i)
        {
            if (levelHandler != null)
            {
                WaterLevelArgs waterArgs=new WaterLevelArgs(i);
                levelHandler(this,waterArgs);
            }
        }
   }

    // WaterLevelArgs class stores the level of
    // the water
    public class WaterLevelArgs : EventArgs
    {
        public int Level
        {
            get;
            set;
        }
        public WaterLevelArgs(int level)
        {
            Level = level;
        }
    }

    class WaterLevelSimulator
    {

        static void Main()
        {
            WaterLevelListener lst = new WaterLevelListener();
            WaterTank tnk = new WaterTank(lst);
            Thread thd1 = new Thread(new ThreadStart(tnk.ConsumeWater));
            Thread thd2 = new Thread(new ThreadStart(tnk.FillWater));
            thd1.Start();
            thd2.Start();
            Console.ReadKey();
        }
    }

}

Questions :

1)Is the above code is thread safe ?

2)As C# 2.0 and 3.0 introduced Action<>,Predicate<>,lambdas how can i improve my code?

3) What is the best pattern can i follow in order to use publisher ,Observer pattern,I mean should i need to design separate class for (i) custom EventArgs (ii) Listeners (iii) publishers (iv) linker -(linkiing listeners,publishers,custom EventArgs) ?

Answer 1

Is this homework? Anyways...

1) After a quick look over it, the code looks threadsafe to me, but so heavily synchronized that the threads actually do not perform any work simultaneously. Therefore, for this code you could just as well have one thread with a loop.

2) Don't worry about that now, a real improvement doesn't come from syntax only (and in this case that's mainly what those constructs would be getting you).

3) It's not clear to me what you need there.

Answer 2

You should read the following article about locking objects (and follow the link in the comment on top of the article):

http://haacked.com/archive/2005/04/12/NeverLockThis.aspx

Answer 3

Using lock(this) (also Monitor.Wait(this)) is not recommended by Microsoft (see MSDN on lock-Keyword).

You sould use a separate locking object, like a string variable or something else that's an object - this won't work for simple types like int or something.