How to make LINQ statement ThreadSafe?

[GregSenne]

I have a LINQ statement that returns only certain objects from a collection. I keep getting "collection has been modified" errors, because other threads are modifying the "Checks" collection at the same time that this LINQ statement is trying to pull some numbers.

Here's the procedure where I keep getting the "collection was modified" errors:

Public Function GetActualCheckCount() As Int32
       
        Dim RetryCount As Int32 = 0
Retry:  Try 
 
             Return (From x In Checks Where Not x.Status = 3).Count
 
        Catch ex0708 As Exception
            If ex0708.Message.ToLower.Contains("collection was modified") AndAlso RetryCount < 8 Then
                RetryCount += 1
                GoTo Retry
            End If
        End Try
    End Function

[Shaggy Hiker]

I'm not even sure if you CAN make a LINQ statement thread safe in a general sense. The problem is that the LINQ won't get executed, and therefore the code won't actually be executed, until it is needed. You would have to lock the common objects. In the specific case you show, that should be ok, since you are using .Count, so the LINQ should be executed immediately, and you'd only have to lock and release around that one statement. In cases where the LINQ may not be evaluated right away, you'd have to lock before the statement was executed, then release after the statement, which would require you to know when the statement would execute.

Furthermore, you can't simply change the code you showed and make everything ok. It doesn't help all that much doing synchronization on only one thread when the other threads don't bother. Each thread has to lock and release the shared resource if you want them all to play nicely together.

In this case, there may be another option. You ultimately want only an integer. Suppose that each thread that modified the collection determined whether or not the change it was making would affect the number. If you did that, each thread would be able to alter a common integer (though that would have to be done in a thread safe manner, too), and the method you are showing wouldn't have to deal with thread safety since it would only be reading that integer.

[GregSenne]

When you say "lock", do you mean something like this:

Public Function GetActualCheckCount() As Int32
           
            Dim RetryCount As Int32 = 0
    Retry:  Try
                 SyncLock Checks
                      Return (From x In Checks Where Not x.Status = 3).Count
                  End SyncLock
     
            Catch ex0708 As Exception
                If ex0708.Message.ToLower.Contains("collection was modified") AndAlso RetryCount < 8 Then
                    RetryCount += 1
                    GoTo Retry
                End If
            End Try
        End Function

[Shaggy Hiker]

Yeah, SyncLock is probably the easiest of all options to use. However, it won't do anything for you unless all the threads lock the object before making any changes to it. That's the only safe way to go anyhow.

[szlamany]

@shaggy - does that mean if I use synclock to lock a resource on my UI thread - from a background thread - that I also need to do it in the UI thread when I access that same object?

I'm not sure I was really clear on that if it is the case...

[Shaggy Hiker]

Yes. Any resource that will be accessed for anything other than reading by multiple threads has to be locked before use to be safe. This can be as simple as a line like:

a += 1

If a is accessible, and altered, by more than one thread, then that line has to be protected on each thread. If only one thread is locking it, while another is not, then the one that is not is going to ignore the lock by the other thread and go right ahead and access the resource. Of course, there are situations where locking isn't necessary, such as where a thread is ONLY reading a value type variable, or cases where a resource is accessible to other threads, but the other threads aren't going to touch it. These are really common situations. For all others, you really need to use protection to ensure that all the threads play nicely, or else you can get some fantastically bizarre race condition bugs. For instance, the program could work correctly for months, then suddenly fail one day for reasons that you can't ever explain or replicate. The safe thing to do is to lock a common resource before doing anything with it, but only hold the lock for as little time as possible. It's that last part that can make LINQ problematic, since the lock could be acquired prior to the LINQ statement, yet the actual code generated for the LINQ statement may not be executed until later (unless you use something like .Count that will cause it to execute immediately), and that could necessitate holding the lock longer than absolutely necessary. Of course, that will only cause performance degradation, and that degradation may be insignificant, so it is always a bit of a thought puzzle.

[Shaggy Hiker]

Yes. Any resource that will be accessed for anything other than reading by multiple threads has to be locked before use to be safe. This can be as simple as a line like:

a += 1

If a is accessible, and altered, by more than one thread, then that line has to be protected on each thread. If only one thread is locking it, while another is not, then the one that is not is going to ignore the lock by the other thread and go right ahead and access the resource. Of course, there are situations where locking isn't necessary, such as where a thread is ONLY reading a value type variable, or cases where a resource is accessible to other threads, but the other threads aren't going to touch it. These are really common situations. For all others, you really need to use protection to ensure that all the threads play nicely, or else you can get some fantastically bizarre race condition bugs. For instance, the program could work correctly for months, then suddenly fail one day for reasons that you can't ever explain or replicate. The safe thing to do is to lock a common resource before doing anything with it, but only hold the lock for as little time as possible. It's that last part that can make LINQ problematic, since the lock could be acquired prior to the LINQ statement, yet the actual code generated for the LINQ statement may not be executed until later (unless you use something like .Count that will cause it to execute immediately), and that could necessitate holding the lock longer than absolutely necessary. Of course, that will only cause performance degradation, and that degradation may be insignificant, so it is always a bit of a thought puzzle.