2
\$\begingroup\$

I need to create thread-safety on a Web-API method, but only when two requests containing the same property value are ran simultaneously.

I have created a mix of WaitHandles and a ConcurrentDictionary to only hold the threads that have another thread processing a similar item.

Is this code thread-safe? The one that worries me is the ContainsKey with the TryAdd.


private readonly static ConcurrentDictionary _processingGateways = new ConcurrentDictionary();

public Models.Profile GetProfile([FromBody] GetProfileRequest request)
{
    if (!ModelState.IsValid)
        throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.BadRequest, new HttpError(ModelState, true)));

    if (_processingGateways.ContainsKey(request.GatewayId))
        _processingGateways[request.GatewayId].WaitOne();
    _processingGateways.TryAdd(request.GatewayId, new AutoResetEvent(false));

    // Wait for this until prev with same gatewayId has finished
    var device = _clientRegistrar.RegisterMobileEndpoint(Mapper.Map(request));

    AutoResetEvent currentHandle;
    _processingGateways.TryRemove(request.GatewayId, out currentHandle);
    currentHandle.Set();

    return Mapper.Map(device);
}
\$\endgroup\$
3
\$\begingroup\$

According to MSDN, all public and protected members of ConcurrentDictionary<TKey, TValue> are thread-safe and may be used concurrently from multiple threads. However, concurrency only applies to the individual actions performed on the instance of the ConcurrentDictionary. The situation will not get messy, if a thread is performing a write operation and an another thread is performing a read operation at the same time. But, there is no protection within the ConcurrentDictionary class against the situation, when an information of the past is provided, that has since changed and because of that, an invalid operation is performed.

I do not see the reason of calling ContainsKey, since an another thread could remove that particular element between the ContainsKey and the Dictionary value accessor operations and KeyNotFoundException will be thrown.

if (_processingGateways.ContainsKey(request.GatewayId))
// the element could be removed right at this point during execution
    _processingGateways[request.GatewayId].WaitOne(); // KeyNotFoundException is thrown here
_processingGateways.TryAdd(request.GatewayId, new AutoResetEvent(false));

The key to your solution could be the TryGetValue method. It is going to lock the ConcurrentDictionary for reading and return the element (if it exists).

if (_processingGateways.TryGetValue(request.GatewayId, out var Gateway))
    Gateway.WaitOne();
_processingGateways.TryAdd(request.GatewayId, new AutoResetEvent(false));

(Disclaimer: code uses C#7.0 out variables)

Now your are secured against the exception.

If you inspect the code for possible deadlock situations, you might observe that it is possible, that Set gets executed before WaitOne. It is good to know, that the wait returns when all events are set and it does not matter, whether or not that happened before or after wait was called. Great, no deadlocks here!

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.