Thread-safe removewhere I'm trying to implement a HashSet that it's accessed from different thread for querying, adding or deleting elements. It doesn't make sense to use a Dictionary for I got several search keys so I won't get benefited much for the speed provided on querying.

My real problem is when trying to remove several items matching a given definition without locking the List for to long, I use this code right now

   public static void DeleteFromHashSet(string companyId, int deviceId)
   {
        lock (setLock)
        {
            set.RemoveWhere(x =&gt; x.CompanyId == companyId &amp;&amp; x.y == deviceId);
        }
    }


but this has the problem of locking the access for querying the hashset for the duration of the RemoveWhere, I can change that with a foreach and just lock for each individual item but I wanted to know if there's a better way.

Update:

This is what I finally did, in case it's of some help

    class DeviceLocator
{
    #region  fields

    private HashSet&lt;DeviceLocator&gt; m_deviceList = new HashSet&lt;DeviceLocator&gt;();
    private object setLock = new object();
    private ReaderWriterLockSlim cacheLock = new ReaderWriterLockSlim();

    public  void AddLocator(DeviceLocator locator)
    {
        DeleteLocator(locator);

        cacheLock.EnterWriteLock();

        m_deviceList.Add(locator);

        cacheLock.ExitWriteLock();
    }

    public void AddLocators(IEnumerable&lt;DeviceLocator&gt; locators)
    {
        foreach (DeviceLocator locator in locators)
        {
            AddLocator(locator);
        }
    }

    #endregion Add items

    #region Query items

    public DeviceLocator GetLocator(string serialNumber)
    {
        try
        {
            return ReadSafely(x =&gt; x.SerialNumber == serialNumber).Single();
        }
        catch
        {
            throw new LocatorNotFoundException(ExceptionTexts.LocatorNotFoundException, null, ImportanceLevel.Low, m_ID);
        }
    }

    public IEnumerable&lt;DeviceLocator&gt; GetLocatorsFromCompany(string companyId)
    {
        return ReadSafely(x =&gt; x.CompanyID == companyId);
    }

    public IEnumerable&lt;string&gt; GetLocatorsStringFromCompany(string companyId)
    {
        return GetLocatorsFromCompany(companyId).Select(y =&gt; y.SerialNumber);
    }

    public IEnumerable&lt;DeviceLocator&gt; GetsLocatorBasedOnConnectionType(ConnectionType type)
    {
        return ReadSafely(x =&gt; x.ConnectionType == type); ;
    }

    private IEnumerable&lt;DeviceLocator&gt; ReadSafely(Func&lt;DeviceLocator, bool&gt; predicate)
    {
        try
        {
            cacheLock.EnterReadLock();

            var result = m_deviceList.Where(predicate);

            return result;
        }
        catch
        {
            throw;
        }
        finally
        {
            cacheLock.ExitReadLock();
        }
    }

    public  void DeleteLocator(DeviceLocator locator)
    {
        DeleteSafely(x =&gt; x.CompanyID == locator.CompanyID &amp;&amp; x.SerialNumber == locator.SerialNumber);
    }

    /// &lt;summary&gt;
    /// Removes all locators from given company
    /// &lt;/summary&gt;
    /// &lt;param name="companyId"&gt;&lt;/param&gt;
    public  void DeleteLocator(string serialNumber)
    {
        DeleteSafely(x =&gt; x.SerialNumber == serialNumber);
    }

    public  void DeleteLocatorsFromCompany(string companyId)
    {
        DeleteSafely(x =&gt; x.CompanyID == companyId);
    }

    private  void DeleteSafely(Predicate&lt;DeviceLocator&gt; predicate)
    {
        try
        {
            cacheLock.EnterWriteLock();

            m_deviceList.RemoveWhere(predicate);
        }
        catch
        {
            throw;
        }
        finally
        {
            cacheLock.ExitWriteLock();
        }
    }
}
Firstly, the biggest problem is that ReaderWriterLockSlim implements IDisposable so your DeviceLocator class should too as it holds a reference at the class level.



You could make better use of the locks if you moved things round a bit:

public void AddLocators(IEnumerable&lt;DeviceLocator&gt; locators)
{
    cacheLock.EnterWriteLock();
    foreach (DeviceLocator locator in locators)
    {
        AddLocatorUnsafe(locator);
    }
    cacheLock.ExitWriteLock();
}

private void AddLocatorUnsafe(DeviceLocator locator)
{
    m_deviceList.RemoveWhere(x =&gt; x.CompanyID == locator.CompanyID 
            &amp;&amp; x.SerialNumber == locator.SerialNumber);

    m_deviceList.Add(locator);
}


Now you only acquire the lock once and do everything while holding that lock instead of having to keep acquiring and releasing between items.



Don't do this:

try
{
    // ...
}
catch
{
   throw;
}
finally
{
    // ...
}


That catch block is entirely redundant - remove it.



Some people would probably tell you off for using regions. I'm not really that fussed about it but you should be aware that a lot of C# developers don't like using regions for splitting up classes.