Newtonsoft Json serializer Getting NullReferenceException when using CamelCasePropertyNamesContractResolver

Question

I'm using Newtonsoft to serialize objects to Json with the CamelCasePropertyNamesContractResolver Contract Resolver.

Once in while (randomly) Newtonsoft is getting a NullReferenceException, when trying to resolve contract, and then all of the rest of serialization is failing.

I'm using .Net core 2.1 on Linux machine, and Newtonsoft version 11.0.2. Contract resolver type is CamelCasePropertyNamesContractResolver.

error

ex=System.NullReferenceException: Object reference not set to an instance of an object.
 at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
 at System.Collections.Generic.Dictionary`2..ctor(IDictionary`2 dictionary, IEqualityComparer`1 comparer)
 at Newtonsoft.Json.Serialization.CamelCasePropertyNamesContractResolver.ResolveContract(Type type)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.CalculatePropertyValues(JsonWriter writer, Object value, JsonContainerContract contract, JsonProperty member, JsonProperty property, JsonContract& memberContract, Object& memberValue)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty)
 at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType)
 at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType)

The code that serialize the object

if (serializerSettings == null)
    serializerSettings = new CamelCaseSerializerSettings();
var serializer = JsonSerializer.Create(serializerSettings);
using (var streamWriter = new StreamWriter(writeTo,  DEFAULT_ENCODING, 1024*2, true))
using (var jsonTextWriter = new JsonTextWriter(streamWriter))
{​​​​​​​
   jsonTextWriter.CloseOutput = false;
   serializer.Serialize(jsonTextWriter, data);
}​​​​​​​

public CamelCaseSerializerSettings(Formatting serializerFormatting = Formatting.Indented, JsonConverter converter = null)
{​​​​​​​
   Formatting = serializerFormatting;
   ContractResolver = new CamelCasePropertyNamesContractResolver();
   if (converter != null)
   {​​​​​​​
       Converters.Add(converter);
   }​​​​​​​
}​​​​​​​

• Why is this error happening?
• How can I fix it?

An issue was filed about this here: Newtonsoft Json serializer Getting NullReferenceException when using CamelCasePropertyNamesContractResolver #2507.

Answer 1

This may be a threading bug with Newtonsoft's CamelCasePropertyNamesContractResolver. This contract resolver shares contract information globally across all instances of the same resolver type, using its own inline implementation of a thread-safe dictionary caching mechanism, as can be seen in the reference source. Notably, this caching mechanism differs from the ThreadSafeStore<TKey, TValue> used elsewhere in Json.NET including DefaultContractResolver. The exception you are seeing happens when Json.NET tries to construct a new static cache from a pre-existing cache in order to add a new contract.

You may wish to report an issue about this to Newtonsoft, the problem might resolve itself if CamelCasePropertyNamesContractResolver were to use ThreadSafeStore<TKey, TValue> for caching. E.g. the latter uses memory barriers inside its AddValue() method while the camel case resolver's inline implementation does not use memory barriers.

As a potential workaround, consider replacing CamelCasePropertyNamesContractResolver with an appropriately configured static instance of DefaultContractResolver:

static IContractResolver camelCaseResolver = new DefaultContractResolver 
{ 
    // Set ProcessDictionaryKeys and OverrideSpecifiedNames as per your preference.  These are the settings used by CamelCasePropertyNamesContractResolver
    NamingStrategy = new CamelCaseNamingStrategy { ProcessDictionaryKeys = true, OverrideSpecifiedNames = true } 
};

public static IContractResolver CamelCaseResolver => camelCaseResolver;

public static JsonSerializerSettings CreateCamelCaseSerializerSettings(Formatting serializerFormatting = Formatting.Indented, JsonConverter converter = null)
{
    var settings = new JsonSerializerSettings
    {
        ContractResolver = CamelCaseResolver,
        Formatting = serializerFormatting,
    };
    if (converter != null)
        settings.Converters.Add(converter);
    return settings;
}

Newtonsoft recommends caching the contract resolver for best performance so I do as well.

---

Update

After reviewing:

• Can another thread see partially created collection when using collection initializer?
• C# - The C# Memory Model in Theory and Practice
• Is volatile still needed inside lock statements?

I believe there may be one or more threading bugs in CamelCasePropertyNamesContractResolver.ResolveContract():

private static readonly object TypeContractCacheLock = new object();
private static Dictionary<StructMultiKey<Type, Type>, JsonContract>? _contractCache;

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)
        {
            // Bug: No checking to see whether the cache contains the contract inside the lock, which is needed in case `_contractCache` was modified by another thread between the outer check and the lock, or the value of `_contractCache` was stale.
            // Possible bug: no volatile read.
            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;

            // Bug: no Thread.MemoryBarrier()
            // Possible bug: no volatile write.
            _contractCache = updatedCache;
        }
    }

    return contract;
}

The possible bugs include:

• There is no Thread.MemoryBarrier() before _contractCache is reset. This allows the compiler to reorder the code as follows:

           _contractCache = updatedCache;
           updatedCache[key] = contract;
  

  Which would allow unlocked reader threads to access the updatedCache as it is being modified, possibly causing an exception.

  Newtonsoft's ThreadSafeStore<TKey, TValue> does this correctly.

• There is no double check locking to see whether the required contract was added to the cache inside the lock.

  Once again ThreadSafeStore<TKey, TValue> does this correctly.

• Possibly _contractCache needs to be volatile.

  I am not sure this is necessary since it is only ever mutated from inside the lock statement. It may be sufficient to add double check locking as mentioned above.

  ThreadSafeStore<TKey, TValue> does not use volatile for its internal cache.

Switching to a static DefaultContractResolver should resolve these issues as it uses ThreadSafeStore<TKey, TValue> internally.