C# – Concurrent collections, should read methods allow multiple threads at one time

ccollectionsmultithreading

I have custom collection and I want to add wrapper to allow concurrent access.

public class MyConcurrentCollection<T>
{
    private MyCollection _collection; // passed in constructor

    public void Add(T item)
    {
        //modifies and may temporarily "break" the collection while executing this method
    }

    public bool Contains(T item)
    {
        //only reads
    }

    // other read and write methods
}

At this moment, I have object private member that acts as lock in every method, allowing only one thread at a time to access the collection, so every method looks like this:

public bool Contains(T item)
{
    lock(_lock)
    {
        return _collection.Contains(item);
    }
}

However this seems really inefficient. Since Contains() only reads from the collection, should I allow multiple threads into it?

Of course, I need to lock access to Add() and other methods while there are threads in Contains() and I need to block access to Contains() if there is a thread wishing to modify the collection.

Is there any disadvantage of allowing multiple threads into read only methods, or should I stick with my basic solution?

Best Answer

It is inefficient but it is necessary - what happens if you read from the collection while someone else is adding a new entry but, in the way of threading, hasn't quite finished writing the new entry's data?

There are ways to make it more efficient, particularly using a read-write lock, which locks the entire collection to both readers and writers if someone is writing, but allows multiple readers access (ie a read lock prevents a writer - the writer has to wait until you're done reading, but does not block other readers).

Incidentally, using an object as a lock is not considered best practice. Use a dedicated lock construct. I remember reading somewhere the CLR team wish they'd never allowed such use.

Related Solutions

C# – Clients with multiple proxy and multithreading callbacks

I created a SingleTon class that can be used to Manage this Proxy.

/// <summary>
/// Singleton Pattern
/// <para>ConnectionHelper provides connection to the Server.</para>
/// </summary>
public sealed class ConnectionHelper
{
    private static readonly ConnectionHelper instance = new ConnectionHelper();
    /// <summary>
    /// Provides a readonly Instance to "ConnectionHelper" class.
    /// </summary>
    public static ConnectionHelper Instance
    {
        get
        {
            return instance;
        }
    }
    /// <summary>
    /// Provides access to IServer members.
    /// </summary>
    public ServerProxy ServerProxyInstance { get; set; }

    /// <summary>
    /// Gets or sets the peer vue communication URL.
    /// </summary>
    /// <value>The peer vue communication URL.</value>
    public string CommunicationURL
    {
        get;
        set;
    }

    .....
    ...
    ..
}

Java.util.HashMap lock on actual HashMap object compare to lock on object that encapsulate the HashMap

Encapsulation is necessary here in order to restrict direct access of client code to methods of HashMap.

As for recommendation on what object to use as a lock, it serves a purpose to allow client code to iterate over the map in a thread safe way.

If you intend your map to be synchronized but lock on actual HashMap object, without hiding (encapsulating) it, then any code having access to that object would be able to invoke its unsynchronized methods and break synchronization. Purpose of encapsulation in this case is stated further in javadocs you refer: "to prevent accidental unsynchronized access to the map".

Documentation also refers to Collections.synchronizedMap as an example on how one is expected to synchronize:

Returns a synchronized (thread-safe) map backed by the specified map. In order to guarantee serial access, it is critical that all access to the backing map is accomplished through the returned map...

You see, synchronizedMap documentation reiterates the importance ("it is critical") for the client code not accessing "backing map" directly.

This documentation also further provides the example explaining why it is recommended to synchronize on the enclosing object:

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views:
  Map m = Collections.synchronizedMap(new HashMap());
      ...
  Set s = m.keySet();  // Needn't be in synchronized block
      ...
  synchronized (m) {  // Synchronizing on m, not s!
      Iterator i = s.iterator(); // Must be in synchronized block
      while (i.hasNext())
          foo(i.next());
  }
Failure to follow this advice may result in non-deterministic behavior.

You see, in the above code example, if client code would not have access to "known by convention" lock object, it would be impossible to properly synchronize: some other code could have used other lock to wrap the iteration which would lead to unsynchronized access.

Best Answer

Related Solutions

C# – Clients with multiple proxy and multithreading callbacks

Java.util.HashMap lock on actual HashMap object compare to lock on object that encapsulate the HashMap

Related Topic