dbc2
commented
3 years ago This issue might resolve itself it CamelCasePropertyNamesContractResolver were modified to use a ThreadSafeStore<StructMultiKey<Type, Type>, JsonContract> for contract caching instead of implementing its own thread-safe locking for a Dictionary<StructMultiKey<Type, Type>, JsonContract>? _contractCache;.
The reason I suspect this is that ThreadSafeStore<TKey, TValue>.AddValue(TKey key) has a memory barrier just before the static _store is reset:
#if !HAVE_CONCURRENT_DICTIONARY
private TValue AddValue(TKey key)
{
TValue value = _creator(key);
lock (_lock)
{
if (_store == null)
{
_store = new Dictionary<TKey, TValue>();
_store[key] = value;
}
else
{
// double check locking
if (_store.TryGetValue(key, out TValue checkValue))
{
return checkValue;
}
Dictionary<TKey, TValue> newStore = new Dictionary<TKey, TValue>(_store);
newStore[key] = value;
#if HAVE_MEMORY_BARRIER
Thread.MemoryBarrier();
#endif
_store = newStore;
}
return value;
}
}
#endifHowever, CamelCasePropertyNamesContractResolver has no equivalent barrier:
public override JsonContract ResolveContract(Type type)
{
if (type == null)
{
throw new ArgumentNullException(nameof(type));
}
// for backwards compadibility the CamelCasePropertyNamesContractResolver shares contracts between instances
StructMultiKey<Type, Type> key = new StructMultiKey<Type, Type>(GetType(), type);
Dictionary<StructMultiKey<Type, Type>, JsonContract>? cache = _contractCache;
if (cache == null || !cache.TryGetValue(key, out JsonContract contract))
{
contract = CreateContract(type);
// avoid the possibility of modifying the cache dictionary while another thread is accessing it
lock (TypeContractCacheLock)
{
cache = _contractCache;
Dictionary<StructMultiKey<Type, Type>, JsonContract> updatedCache = (cache != null)
? new Dictionary<StructMultiKey<Type, Type>, JsonContract>(cache)
: new Dictionary<StructMultiKey<Type, Type>, JsonContract>();
updatedCache[key] = contract;
// No Thread.MemoryBarrier(); !!
_contractCache = updatedCache;
}
}
return contract;
}I don't really understand why a memory barrier is needed when the code is already inside a lock statement -- but the inconsistency is clear. Replacing the inline locking with a ThreadSafeStore<StructMultiKey<Type, Type>, JsonContract> looks to be the simplest way to resolve the inconsistency. (This will also make use of ConcurrentDictionary<TKey, TValue> if available, which might also improve performance.)
See: this answer to Newtonsoft Json serializer Getting NullReferenceException when using CamelCasePropertyNamesContractResolver.
DanamoCP
elgonzo
Source/destination types
CamelCase Serializer
Expected behavior
object being serialized
Actual behavior
Newtonsoft is occasionally getting a NullReferenceException, when trying to resolve contract.
stack trace:
Steps to reproduce