CODEWITHSUNDEEP
asp.netvb.net
Shawn
Shawn

Reputation: 19803

Is this code thread safe?

''' <summary>
''' Returns true if a submission by the same IP address has not been submitted in the past n minutes.     
'' </summary>
Protected Function EnforceMinTimeBetweenSubmissions(ByVal minTimeBetweenRequestsMinutes  as Integer) As Boolean       
    If minTimeBetweenRequestsMinutes = 0 Then
        Return True
    End If
    If Cache("submitted-requests") Is Nothing Then            
        Cache("submitted-requests") = New Dictionary(Of String, Date)
    End If
    ' Remove old requests. '
    Dim submittedRequests As Dictionary(Of String, Date) = CType(Cache("submitted-requests"), Dictionary(Of String, Date))
    Dim itemsToRemove = submittedRequests.Where(Function(s) s.Value < Now).Select(Function(s) s.Key).ToList
    For Each key As String In itemsToRemove
        submittedRequests.Remove(key)
    Next
    If submittedRequests.ContainsKey(Request.UserHostAddress) Then
        ' User has submitted a request in the past n minutes. '
        Return False
    Else            
        submittedRequests.Add(Request.UserHostAddress, Now.AddMinutes(minTimeBetweenRequestsMinutes))
    End If
    Return True
End Function

Upvotes: 0

Views: 160

Answers (2)

Justin Niessner
Justin Niessner

Reputation: 245419

No. The ASP.NET Cache is not inherently thread-safe and it looks like you are creating objects in the Cache depending on whether they exist or not.

You need to lock the Cache when writing to it.

Let me word things a little differently. The code is, in fact, thread safe. The way you currently have it coded though could cause performance issues in multi-threaded situations.

In this case, multiple users would be running the same code simultaneously, theoretically accessing and modifying the same cache objects at the same time. As that scenario scales up, performance suffers.

Creating a lock will improve performance under heavy load (while imposing a slight overhead under light load) because you won't be fetching data neadlessly due to Caching issues.

Upvotes: 3

Brian Gideon
Brian Gideon

Reputation: 48949

The System.Web.Caching.Cache class is thread-safe according to the MSDN documenation. However, the documenation also shows an example where a read and a write are performed on the cache without locking. That cannot possibily be thread-safe since the write is dependent on the read. The code you posted basically looks like the example. I definitely recommend putting a lock around the entire method.

Upvotes: 1

Related Questions