Multi Threading locks and monitor class not working

Question

I have a file that is read and written to. I need to make sure when its been written to, nobody else will try to write to it.

I put a lock on the whole function which allows to either read or write but I still get errors such as The process cannot access the file 'FILENAME' because it is being used by another process.

public static TYPE Property{

get{
    data mydata;
    Object obj = new object();
    Monitor.Enter(obj);

    // check if data has not changed
    // if it has not, just read

    using (Stream stream = File.Open(fileLocation, FileMode.Open, FileAccess.Read, FileShare.ReadWrite)) {
        //....
    }
    // else, if data changed, then need to write to  file to save the new data

    using (Stream stream = File.Open(fileLocation, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.Read)) {
        BinaryFormatter bf = new BinaryFormatter();

        try {
            bf.Serialize(stream, (data);
        }
        //DONE processing

        Monitor.Pulse(obj);
        Monitor.Exit(obj);
        return data
}

Answer 1

You're creating a new monitor to lock on every time the property is invoked. You need to lock on the same monitor otherwise there's no point in locking at all.

You should also just use a "lock" statement - you're never waiting, so there's no point in pulsing. Currently, if any exceptions are thrown, you'll end up "leaking" the lock. That would normally be a really bad problem, but as you're not reusing the lock anyway, that's masking the issue.

For example:

private static readonly object monitor = new object();

public static TYPE Property
{
    get
    {
        lock(monitor)
        {
            // check if data has not changed
            // if it has not, just read
            using (Stream stream = File.Open(fileLocation, 
                   FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
            {
                ....
            }
            // else, if data changed, then need to write to 
            // file to save the new data
            using (Stream stream = File.Open
                       (fileLocation, FileMode.OpenOrCreate,
                        FileAccess.ReadWrite, FileShare.Read))
            {
                BinaryFormatter bf = new BinaryFormatter();
                bf.Serialize(stream, data);
            }
            return data;
        }
    }
}

As an aside, this looks like it's doing more work than I'd really expect in a property. Are you sure a method wouldn't make more sense?

Answer 2

Well, Monitor.Enter blocks access of any threads trying to place a lock on THE SAME object. Every time you enter your getter you create a new object, so every caller gets a new lock which know nothing about each other.

In other words there is no locking.

as a side note - why you do not use the lock statement? You will still need a global lock object.

Answer 3

The reason for the global variable versus your local variable is that the LOCK is actually locking down a point of reference in memeory as I understand it. Every time you instantiate a new object, i.e. "Object obj = new object();", you're creating a new object, with it's own unique pointer in memory. So when LOCK looks to see if the point in memory is locked down, it's not. Because it's a brand new referenced point in memory and the only one using it is the caller entering your property. With having your Obj variable declared globally, it will always be the same point in memory and lock can actually validate, that indeed, that point in memory is either currently locked or it can lock it it self.

Example: (crude but I think it gets the point)

Object obj = new object();

Now you have a point in memory that looks kinda like:

Memory -

    * obj1

now you enter your property again and create a new object yet again. Your system memory now kinda looks like...

Memory -

   * obj1
   * obj2

On the first trip, your lock is checking "Obj1" in memory. Since the caller of the 1st trip into your property is the only one using that instance of Obj, it's the only one who ever would lock or check it for locking. Because it's looking at that copy of that Obj reference in memory.

On the second trip, your lock is checkign "Obj2" in memory.

When you use a global variable, that point in memory persists so lock always checks the same point in memory. it never moves and it's always the same referenced point in memory. So all callers of your property will always be using the same refernece point in memory, your lock will succeed as you expect it.

Special Note: I'm not stating a life time of "obj". I'm simply stating how this is functiong in a multi threaded process. hope that helps.