Concurrency Bug

[davisnw]

NClone appears to have a concurrency bug.

I recently modified our test suite to run more tests in parallel, and since then I've been getting intermittent failures when cloning. This is using version 1.1.1 from Nuget.

So for example, i I have:

  private Currency _depositAmount = NClone.Clone.ObjectGraph(Currency.Empty);

Then I will sometimes see the exception:

Result Message:

Test method threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
Result StackTrace:	
at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at NClone.ReplicationStrategies.ReplicationStrategyFactory.StrategyForType(Type type)
   at NClone.ObjectReplication.ReplicationContext.Replicate(Object source)
   at NClone.ObjectReplication.ObjectReplicator.Replicate[T](T source)
   at DomainModel.Entities.Deposit.ctor()

From the stacktrace, I suspect this bug is occurring because the replication strategy is caching per type, but using the non-threadsafe System.Collections.Generic.Dictionary. I suspect replacing with a thread-safe collection would resolve the issue.

[davisnw]

There are actually cares where it appears NClone gets stuck in an infinite loop. I'll try to create a minimal example to demonstrate the issue. I think both the NRE and infinite looping are related to the same issue of the internal dictionary not being thread-safe. I believe the issues are likely to arise when NClone is concurrently asked to clone the same type on different threads before the internal cache for that type has been fully constructed.

[davisnw]

The below program (tested targeting .NET Framework 4.5.2 with NClone version 1.1.1 from Nuget) consistently reproduces both the NRE and the infinite looping. The NRE happens almost every time, the infinite looping less often but still frequently enough to see the issue.

using System;
using System.Linq;
using System.Threading.Tasks;

namespace NCloneBug
{
    class Program
    {
        public class ValueObject
        {
            private readonly int _blah;

            private ValueObject() : this(Int32.MinValue){}

            public ValueObject(int blah)
            {
                _blah = blah;
            }

            public static readonly ValueObject Empty = new ValueObject();
        }

        public class Entity
        {

            private ValueObject _foo1 = NClone.Clone.ObjectGraph(ValueObject.Empty);
            public ValueObject Foo1
            {
                get { return _foo1; }
                set {_foo1 = value; }
            }

            private ValueObject _foo3 = NClone.Clone.ObjectGraph(ValueObject.Empty);
            public ValueObject Foo3
            {
                get { return _foo3; }
                set { _foo3 = value; }
            }
        }
  
        static void Main(string[] args)
        {
            Parallel.ForEach(Enumerable.Repeat<Func<Entity>>(() => new Entity(), 20), x =>
            {
                try
                {
                    x();
                }
                catch (Exception e)
                {
                    Console.WriteLine(e.ToString());
                }
            });
        }
    }
}

[mijay]

Thank you for reporting and fixing this one! I'll try to release a new version today.