CODEWITHSUNDEEP
c#multithreadingmonitor
DotNetter
DotNetter

Reputation: 426

Monitor.Pulse() has no effect

Here is fragment of my code. I suppose, that one thread should work without waiting till previous thread is finished. But I recognized that all threads started in series and my pulse call has no effects on way how they work.

class A
{
    string path = "file.xml";
    static public object _lock = new object();
    static private int accessCounter = 0;

    public List<T> GetItems()
    {
        List<T> result = null;
        lock (_lock)
        {

            while (accessCounter < 0)
                Monitor.Wait(_lock);
            accessCounter++;
            Thread.Sleep(1000);

            Monitor.Pulse(_lock);
            Thread.Sleep(1000);

            using (Stream stream = File.OpenRead(path))
            {
                XmlSerializer serializer = new XmlSerializer(typeof(List<T>), new Type[] { typeof(T) });
                result = (List<T>)serializer.Deserialize(stream);
            }
            accessCounter--;
            Monitor.Pulse(_lock);
        }
        return result;
    }
    public void AddItem(T item)
    {
      lock(_lock){if (accessCounter!=0) Monitor.Wait(_lock);
      accessCounter = -1;
      //some writing ooperations
      accessCounter = 0;
       }
    }
}

I know that if condition is useless in current situation because is always true. And more so, they should work simultaneously, but do not.

EDIT: This code called in following way from different threads: 

.....
A a = new A();
var list = a.GetItems();
.....

It should to be some kind of writing/reading block. So, if thread wants to read and some other thread is already reading file, then it does not have to wait other. Now all reading threads is needed to suspend if other thread captured *_lock*

Upvotes: 0

Views: 167

Answers (2)

Iridium
Iridium

Reputation: 23721

The framework already provides a locking mechanism (in fact, two) that should do what you want (i.e. permit many simultaneous readers, but writing is exclusive (no other readers or writers at the same time) - ReaderWriterLock, and ReaderWriterLockSlim.

Your code could be amended to the following to take advantage of ReaderWriterLockSlim:

class A
{
    private readonly ReaderWriterLockSlim _lock = new ReaderWriterLockSlim();
    string path = "file.xml";
    static public object _lock = new object();
    static private int accessCounter = 0;

    public List<T> GetItems()
    {
        _lock.EnterReadLock();
        try
        {
            using (Stream stream = File.OpenRead(path))
            {
                var serializer = new XmlSerializer(typeof(List<T>), new[] { typeof(T) });
                return (List<T>)serializer.Deserialize(stream);
            }
        }
        finally
        {
            _lock.ExitReadLock();
        }
    }

    public void AddItem(T item)
    {
        _lock.EnterWriteLock();
        try
        {
            // Some writing operations
        }
        finally
        {
            _lock.ExitWriteLock();
        }
    }
}

It's also worth noting that as it is this method will leak memory, as you are creating a new XmlSerializer each time using a constructor other than XmlSerializer(Type) or XmlSerializer(Type, String). This will result in a new XML serialization assembly being built and loaded each time, and these assemblies are never freed. You either need to switch to using one of the two constructors mentioned, or caching the XmlSerializer instance (presumably keyed by typeof(T)) so that you do not leak memory.

For more information, see the "Dynamically Generated Assemblies" section here: XmlSerializer documentation

Upvotes: 1

RB.
RB.

Reputation: 37192

Your access counter check is wrong. It should check for greater than, not less than.

while (accessCounter > 0)
    Monitor.Wait(_lock);

However, your code seems a bit odd. The access counter check happens inside a critical thread, locked on _lock, so it should not be possible for more than 1 thread to be executing this code anyway. This is why your code is not executing simultaneously.

Also, be aware that Monitor.Wait releases a lock that the current thread holds - it doesn't wait for a lock to become free.

Upvotes: 1

Related Questions