Is there a thread-safety issue in this ObjectBuilder code?

Question

I suspect a problem with an old version of the ObjectBuilder which once was part of the WCSF Extension project and meanwhile moved into Unity. I am not sure whether I am on the right way or not so I hope someone out there has more competent thread-safety skills to explain whether this could be an issue or not.

I use this (outdated) ObjectBuilder implementation in an ASP.Net WCSF web app and rarely I can see in the logs that the ObjectBuilder is complaining that a particular property of a class cannot be injected for some reason, the problem is always that this property should never been injected at all. Property and class are changing constantly. I traced the code down to a method where a dictionary is used to hold the information whether a property is handled by the ObjectBuilder or not.

My question basically comes down to: Is there a thread-safety issue in the following code which could cause the ObjectBuilder to get inconsistent data from its dictionary?

The class which holds this code (ReflectionStrategy.cs) is created as Singleton, so all requests to my web application use this class to create its view/page objects. Its dictionary is a private field, only used in this method and declared like that:

private Dictionary<int, bool> _memberRequiresProcessingCache = new Dictionary<int, bool>();

private bool InnerMemberRequiresProcessing(IReflectionMemberInfo<TMemberInfo> member)
{
    bool requires;

    lock (_readLockerMrp)
    {
        if (!_memberRequiresProcessingCache.TryGetValue(member.MemberInfo.GetHashCode(), out requires))
        {
            lock (_writeLockerMrp)
            {
                if (!_memberRequiresProcessingCache.TryGetValue(member.MemberInfo.GetHashCode(), out requires))
                {
                    requires = MemberRequiresProcessing(member);
                    _memberRequiresProcessingCache.Add(member.MemberInfo.GetHashCode(), requires);
                }
            }
        }
    }
    return requires;
}  

This code above is not the latest version you can find on Codeplex but I still want to know whether it might be the cause of my ObjectBuilder exceptions. While we speak I work on an update to get this old code replaced by the latest version. This is the latest implementation, unfortunately I cannot find any information why it has been changed. Might be for a bug, might be for performance...

private bool InnerMemberRequiresProcessing(IReflectionMemberInfo<TMemberInfo> member)
{
  bool requires;

  if (!_memberRequiresProcessingCache.TryGetValue(member.MemberInfo, out requires))
  {
    lock (_writeLockerMrp)
    {
      if (!_memberRequiresProcessingCache.TryGetValue(member.MemberInfo, out requires))
      {
        Dictionary<TMemberInfo, bool> tempMemberRequiresProcessingCache =
          new Dictionary<TMemberInfo, bool>(_memberRequiresProcessingCache);
        requires = MemberRequiresProcessing(member);
        tempMemberRequiresProcessingCache.Add(member.MemberInfo, requires);
        _memberRequiresProcessingCache = tempMemberRequiresProcessingCache;
      }
    }
  }
  return requires;
}

Answer 1

1. The use of the hash code looks problematic if you run a very large number of classes / members, as can happen with the singleton approach you mentioned.
2. The double lock was totally odd in the old one (Only one thread goes into the whole section in all cases). Note that locking as the first thing certainly hurts performance. It is a trade of, notice that instead they create a copy to avoid modifying the list as it is being read.