SyncHashtable this[Object key] does not use locking

I went through the implementation of SyncHashtable in defined in .Net framework BCL.

This class provides synchronized access to multiple readers and writers.

One of the methods is implemented as

public override Object this[Object key] {
            get {
                    return _table[key];
            }
            set {
                lock(_table.SyncRoot) {
                    _table[key] = value;
                }
            }
        }

In my opinion the get method should also have a lock on Syncroot before accessing the object.

Consider the scenario :

Thread 1 : Deleting keys from the Hashtable.

Thread 2 : Reading objects using keys.

If a context switch occurs in thread 2 while reading the object and if thread 1 deletes the object , then in such a case the read operation will fail or give inconsistent result.

Hence couldn't we implement this method like this

public override Object this[Object key] {
            get {
                 lock(_table.SyncRoot) 
                  {   
                   return _table[key];
                  }
            }
            set {
                lock(_table.Sy开发者_运维问答ncRoot) {
                    _table[key] = value;
                }
            }
        }

Thanks Vivek

Locking the Hashtable for reading is not necessary because that is already thread safe under these circumstances.

The documentation for Hashtable states:

Hashtable is thread safe for use by multiple reader threads and a single writing thread.

By locking the write access, there is in effect only a single writer, and it is therefore unnecessary to lock reads.

Note that while this is true for Hashtable, it is not the case for Dictionary<TKey, TValue>.

Not knowing the thread-safety guarantees of the Hashtable I believe you are correct as the lock keyword is a monitor not a implicit reader-writer lock. This could definitely cause undesirable effects. However, given the situation as of today I'd look to the Reactive Extensions for .NET 3.5, it ships with a System.Threading assembly that contains the new concurrent classes as part of the .NET 4.0. These classes are much better suited for dealing with multi-threaded access to collections.