2

I achknowledge the fact I have to lock a hashset to make it thread safe when working inside of multiple threads, what I don't really understand is what if you have multiple hashsets? Can you use the same lock object? Or would you need a seperate object?

For example... I have these 2 methods, that use the same object.

private void RemoveSheduledAttack(Attack attack)
{
    lock (_syncRoot)
    {
        _sheduledAttacks.Remove(attack);
    }
}

private void AddActiveAttack(Attack attack)
{
    lock (_syncRoot)
    {
        ActiveAttacks.Add(attack);
    }
}

Declaring:

private readonly HashSet<Attack> _sheduledAttacks;
private static object _syncRoot = new object();
public HashSet<Attack> ActiveAttacks;
xozijow
  • 91
  • 1
  • 1
  • 4
  • 3
    Yes it is safe to use the same lock, or a different lock. It is up to you. Just as long as you lock in such a way that while a `HashSet` is being written to it can't be read to or written from elsewhere. _As a side note, if you are using a `public` `HashSet` then you are in trouble anyway. Even if you lock on `_syncRoot`, any of the consumers of your class can read from or alter `ActiveAttacks` directly and there is nothing you can do to stop them._ – mjwills Oct 27 '17 at 09:22
  • 4
    You can have a single lock object across your entire application if you wanted to - the less granular the lock, the more times you'll be preventing safe parallelism (e.g. if no method ever actually updates both collections at the same time then those methods don't *need* to share a lock but simultaneous calls to such methods are now prevented) – Damien_The_Unbeliever Oct 27 '17 at 09:25
  • See https://stackoverflow.com/questions/157933/whats-the-best-way-of-implementing-a-thread-safe-dictionary?rq=1 – Pedro Duarte Oct 27 '17 at 09:32

1 Answers1

0

Not exactly an answer for your question, but want suggest using existing implementation of concurrent collections.

There are no concurrent implementation for HashSet<T> but you can use ConcurrentDictionary<TKey, TValue>, then your code become little bid cleaner and no need to maintain lock objects

private ConcurrentDictionary<Attack, bool> _sheduledAttacks;
private ConcurrentDictionary<Attack, bool> _activeAttacks;

private void RemoveSheduledAttack(Attack attack)
{
    var dummyValue = false
    _sheduledAttacks.TryRemove(attack, out dummyValue);
}

private void AddActiveAttack(Attack attack)
{
    _activeAttacks.TryAdd(attack, false);
}
Fabio
  • 31,528
  • 4
  • 33
  • 72