-1

Is someList.Count thread safe in C#?

Performance is very important to me and I can not use other thread safe collections due to the complexity of the main program

I know there can be many other cases, but just focus on this simple question:

Is _lock.EnterReadLock necessary for someList.Count or not because it might be an atomic attribute?

private ReaderWriterLockSlim _lock;
private List<SomeObject> _someList;

public void Add(SomeObject obj)
{
    try
    {
        _lock.EnterReadLock();
                
        if (_someList.Count < 10)
        {
            try
            {
                _lock.EnterWriteLock();

                _someList.Add(obj);
            }
            finally
            {
                _lock.ExitWriteLock();
            }
        }
    }
    finally
    {
        _lock.ExitReadLock();
    }
}
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Mehdi
  • 903
  • 3
  • 10
  • 26
  • 4
    99% of the time, you would be best to just use `lock` rather than `ReaderWriterLockSlim`. The code is _much_ simpler to write, and much harder to get wrong. If you then determine there is a performance problem then (and only then) switch to `ReaderWriterLockSlim`. – mjwills Aug 26 '21 at 05:49
  • 1
    Talk us through what `someList` is used for. Why do you need it? Why are things added to it? When and why are they removed from it? – mjwills Aug 26 '21 at 05:50
  • Also, you need to be clear about what you mean by "thread safe". https://stackoverflow.com/a/1352369/34092 suggests it will likely work (for some definition of "work"), but the docs explicitly say it isn't _guaranteed_ - https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=net-5.0#thread-safety . 99% of the time, this means you should `lock`. – mjwills Aug 26 '21 at 06:01
  • 2
    You will not corrupt the data structure just by accessing `Count`. But, of course, if you're not locking all it tells you is that *in the recent past*, the count was a particular number. It doesn't give you any ability to reason about the state of the list *now*. Is that good enough for your purposes? – Damien_The_Unbeliever Aug 26 '21 at 06:29
  • 2
    @Damien_The_Unbeliever It is more accurate to state that _with current implementations_ `You will not corrupt the data structure`. The docs **explicitly** do not promise that going forward. – mjwills Aug 26 '21 at 06:33
  • @Damien_The_Unbeliever So if I understand correctly, there will be no exception by not writing the _lock.EnterReadLock(), but the list.Count may not be expected number, so it is better to use the _lock.EnterReadLock()! Is this true? – Mehdi Aug 26 '21 at 06:34
  • @mjwills It is read at least hundreds of times per second and Maybe only written once a week – Mehdi Aug 26 '21 at 07:58
  • This once a week writing - talk us through it. What does it look like? Since I strongly suspect you can avoid any locking at all if instead of altering the list you created a new one and exchanged it - https://learn.microsoft.com/en-us/dotnet/api/system.threading.interlocked.compareexchange?view=net-5.0#System_Threading_Interlocked_CompareExchange_System_Object__System_Object_System_Object_ . Then all calls to `Count` will be thread-safe, since no writes will be occurring. – mjwills Aug 26 '21 at 08:13
  • @mjwills This is a list of stock identifiers for a group that is updated from time to time, I think we are moving away from the main question – Mehdi Aug 26 '21 at 08:16
  • 2
    We are literally **right at** the main (real, underlying) question. You don't need _any_ explicit locking. You are updating it _once a week_. Use the method I pointed you at. That is all you need. Create a new list once a week. `CompareExchange` it in. Done. No need for _any_ other locking. – mjwills Aug 26 '21 at 08:17
  • But ReaderWriterLockSlim is much faster than the lock, especially in web applications, and in general, the question was, is the lock necessary at all? – Mehdi Aug 26 '21 at 08:21
  • 1
    In your specific case, no locking (`lock` or `ReaderWriterLockSlim`) is needed **if you do what I suggested**. In the general case, some form of locking is a good idea if you are interacting with a type (like `List`) that is explicitly not thread-safe. Now, with the current `List` implementation, you can get away with no locking when reading `Count`. Is this guaranteed for the future? No, no it isn't. – mjwills Aug 26 '21 at 08:23

2 Answers2

1

If you want to be guaranteed that any write operation has completed before you read the count a read lock is required. Assuming that write locks are used for all write actions.

To enter a write lock inside a read lock you have to use EnterUpgradeableReadLock . Calling EnterWriteLock when in a read lock will throw an exception. If you should use a write lock or upgradable lock please read https://stackoverflow.com/a/26578074/9271844.

For more information about the ReaderWriterLockSlim class refer to https://learn.microsoft.com/en-us/dotnet/api/system.threading.readerwriterlockslim?view=net-5.0

gjhommersom
  • 159
  • 1
  • 7
  • By reading https://stackoverflow.com/questions/21411018/readerwriterlockslim-enterupgradeablereadlock-always-a-deadlock, I think the purpose of using the EnterUpgradeableReadLock is only to increase efficiency, not to prevent a deathlock – Mehdi Aug 26 '21 at 07:53
  • But you mentioned a good point to speed up – Mehdi Aug 26 '21 at 08:10
  • 2
    It may be worthwhile highlighting that if locking is used it needs to be used _for all accesses to the list_ (not just getting the `Count`). – mjwills Aug 26 '21 at 08:35
  • 1
    @mehdi I checked the documentation and you are correct that it is not requierd to prevent deadlocks. It is simply not allowed to get a write lock inside a read lock. I'll add your link to the answer. – gjhommersom Aug 26 '21 at 12:54
0

There will be no exception by not writing the _lock.EnterReadLock(), but the list.Count may not be expected number, so it is better to use the _lock.EnterReadLock() if we need the exact list.Count at the moment!

Mehdi
  • 903
  • 3
  • 10
  • 26