-1

Edit: most of the time it works fine the error is exceptional.

On the line where this method is called the error report says it threw a KeyNotFoundException" im mapping an enum to a string (!im not updating the dictionary in any way its filled while getting declared!):

public Task<bool> UpdateStatusAsync(Some object, Status status)
{
    var somerequest = new Somerequest 
    {
        ...
        Status = MapDomainStatusToAclStatus(status.Type),
        ...
    };

    .............
}

Here is the mapping function:

private string MapDomainStatusToAclStatus(DomainStatus domainStatus)
{
    if (DictionaryDomainACLStatus.ContainsKey(domainStatus))
    {
        return DictionaryDomainACLStatus[domainStatus];
    }

    // Some refactor todo comment.
    switch (domainStatus)
    {
        case DomainStatus.Aborted:
            return "Some string";
    }

    Log.Info($"[MapDomainStatusToAclStatus] DomainStatus={domainStatus} cant be mapped to ACL status");
    return String.Empty;
}

Is that even possible?

Edit:

Since i got some replies about possible race condition I would like to add that the dictionary is declared like this:

 public static readonly Dictionary<DomainStatus, string> { values }

Edit2: My dictionary declaration:

public static readonly Dictionary<DomainStatus, string> DictionaryDomainACLStatus= new Dictionary<DomainStatus, string>
    {
            {DomainStatus.Approved, "TEXT" },
            {DomainStatus.Declined, "TEXT2" }
    };

No create update delete operations occur later on in the code.

Viking
  • 465
  • 3
  • 15
  • 2
    The [three most terrifying words](http://thecodelesscode.com/case/225) – Mat J Oct 24 '18 at 06:42
  • 1
    Could it be some other place in the code? – Andrew Oct 24 '18 at 06:50
  • 1
    In short, we have no idea (and can only speculate at the jar of wild and wonderful code you have cooked up). A longer answer is, use a `ConcurrentDictionary` if you are threading – TheGeneral Oct 24 '18 at 06:53
  • 1
    Debugging thoughts: single step through the function and observe the error and parameters; add a try condition to the function? (See @Andrew 's comment) – Peter Smith Oct 24 '18 at 06:54
  • 1
    Between the `if` condition check and the value access, there exists a time where another thread can modify the specific key. A readonly variable can be mutated but cannot be overwritten, so that is of no help here. – Mat J Oct 24 '18 at 07:14
  • 1
    You can try `TryGetValue` instead of Contains check – Mat J Oct 24 '18 at 07:15
  • Why are you using `ContainsKey` rather than `TryGetValue`? Please show a screenshot of the exception as it is occurring, so we can be 100% sure it is occurring where you say it is. What is the **exact** value of `domainStatus` when the exception occurs? When you manually examined the `Dictionary` in the `Watch Window`, did that `domainStatus` value exist as a key in the Dictionary? Please update your question to include the source code for `domainStatus` since if it is implemented poorly this may explain what you are seeing. – mjwills Oct 24 '18 at 08:01
  • Please update your question to show all calls to `DictionaryDomainACLStatus.Add`, `DictionaryDomainACLStatus.Remove` and `DictionaryDomainACLStatus[]` (write calls, not read calls). – mjwills Oct 24 '18 at 08:06
  • I will update my question. Updated!@ – Viking Oct 24 '18 at 08:09
  • Hey mjwills, the rest of the code in that function does not call mapping functions only a wcf servicecall with an object inside. So it has to be that part of the code. But agreed i need more logging. Thanks for the tips.. – Viking Oct 24 '18 at 08:38

3 Answers3

1

Your static dictionary is shared across all threads and therefore not thread safe. Review if it really needs to be static or look at the ConcurrentDictionary which is thread safe.

Darren Lewis
  • 8,338
  • 3
  • 35
  • 55
  • Thanks for your answer. My dictionary is only created (static readonly) and contains 10 items or so. No add or remove or updates take place. Does your answer still apply to that situation? I would understand the problem if my code was updating and removing stuff in the dictionary but since that is not the case im still in the dark. – Viking Oct 24 '18 at 07:53
1

Your problem is that you are within concurrent code, which means that multiple threads of code may run at the same time. So you have to make sure that concurrent operations are atomic, i.e. that that the operation is always executed as one step in a multi-threaded environment and another thread can not interrupt it.

The code...

if (DictionaryDomainACLStatus.ContainsKey(domainStatus))
{
    return DictionaryDomainACLStatus[domainStatus];
}

...is clearly not atomic, because it consists of two separate steps. Between ContainsKey and the indexer, another thread can modify the dictionary, which leads to your KeyNotFoundException if the modification is a Remove.

The first thought coming to mind would be to replace these two steps by...

if (DictionaryDomainACLStatus.TryGetValue(domainStatus, out string value)) {
    return value;
}

This will not help you though, since TryGetValue is in itself not atomic, which means it can be interrupted by another thread internally.

A common solution to this problem is to introduce a critical section with a lock:

lock (myCriticalSection) {
    if (DictionaryDomainACLStatus.ContainsKey(domainStatus))
    {
        return DictionaryDomainACLStatus[domainStatus];
    }
}

Any access of the dictionary has to be in the performed within a lock of the same critical section.

Another option would be to use a ConcurrentDictionary, which provides atomic operations. This will only work though if all accesses to the dictionary can be performed in one (atomic) step. That's why usually you will be better off with a lock.

Sefe
  • 13,731
  • 5
  • 42
  • 55
  • I edited my question with the declaration of my dictionary. I only read from then on. No create update delete operations occur later on in the code. – Viking Oct 24 '18 at 08:12
0

I found out the error lies in another piece of code and I overlooked it. Thank you all for thinking along with me. I will definately be more precise while debugging.

Viking
  • 465
  • 3
  • 15