thread-safety issue with TypeAdapterConfig

[centur]

Today after running a large suit of tests at once I saw one test failed with this exeption

System.ArgumentException
Destination array was not long enough. Check destIndex and length, and the array's lower bounds.
   at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)
   at System.Collections.Generic.List`1.CopyTo(T[] array, Int32 arrayIndex)
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.<ReverseIterator>d__74`1.MoveNext()
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.OrderedEnumerable`1.<GetEnumerator>d__1.MoveNext()
   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Mapster.TypeAdapterConfig.GetMergedSettings(Type sourceType, Type destinationType, MapType mapType)
   at Mapster.TypeAdapterConfig.CreateMapExpression(Type sourceType, Type destinationType, MapType mapType, CompileContext context)
   at Mapster.TypeAdapterConfig.CreateInlineMapExpression(Type sourceType, Type destinationType, MapType mapType, CompileContext context)
   at Mapster.Adapters.BaseAdapter.CreateAdaptExpression(Expression source, Type destinationType, CompileArgument arg)
   at Mapster.Adapters.CollectionAdapter.CreateArraySet(Expression source, Expression destination, CompileArgument arg)
   at Mapster.Adapters.BaseAdapter.CreateBlockExpressionBody(Expression source, Expression destination, CompileArgument arg)
   at Mapster.Adapters.BaseAdapter.CreateExpressionBody(Expression source, Expression destination, CompileArgument arg)
   at Mapster.Adapters.BaseAdapter.CreateAdaptFunc(CompileArgument arg)
   at Mapster.TypeAdapterConfig.CreateMapExpression(Type sourceType, Type destinationType, MapType mapType, CompileContext context)
   at Mapster.TypeAdapterConfig.CreateMapFunction(TypeTuple tuple)
   at Mapster.TypeAdapterConfig.Compile()
   at DBCloud.WebAPI.WebDTOMapSetup.VerifyConfiguration() in C:\Projects\Drawboard.Cloud\DBCloud.WebAPI\App_Start\WebDTOMapSetup.cs:line 159

It never happened before, test is very simple and on a second run this error disappeared.
But after checking the error I found this interesting SO thread

Could it be that Mapster has some locking issues ?
PS: error originates from this call

        public static void VerifyConfiguration()
        {
            TypeAdapterConfig.GlobalSettings.Compile();
        }

[chaowlert]

In fact, Mapster is thread safety for Adapt but setting doesn't. I guess exception occur because unit test call Compile and add new setting at the same time.

To make setting thread safety might be a bit big work, could you please set unit test to run only 1 thread on CI for now?

[centur]

This happened only once, I believe it's pretty rare condition, I haven't seen it before and will keep an eye on it and comment here if this started to happen more often.

Interesting, though, to avoid excessive initialization, our top-level Mapster configuration is being assembled in a static class (I need to see if it's in the constructor or just in method) - building aggregate root in the static class constructor should be pretty thread-safe, as it's called once per app domain and strictly before any usage of this class.

[mikebridge]

I am also hitting what appears to be a thread-safety issue with TypeAdapterConfig. I have what looks like this in one static class:

public static class FacultyMapping
{
    static FacultyMapping()
    {
        TypeAdapterConfig<FacultyViewModel, Faculty>
            .NewConfig()
            .Map(dest => dest.FacultyTags,
               src => MapTagsToFacultyTags(src));
            // ...
    }

    public static Faculty ToFaculty(this FacultyViewModel facultyViewModel)
    {
        return facultyViewModel.Adapt<Faculty>();
    }
}

My test looks like this:

[Fact]
public void It_Maps_FacultyViewModel_To_A_Faculty()
{
    // Arrange
    var facultyViewModel = FacultyFixture.CreateFacultyViewModel(Guid.NewGuid(), Guid.NewGuid());

    // Act
    var faculty = facultyViewModel.ToFaculty();

    // Assert
    Assert.NotNull(facultyViewModel.Tags);
    Assert.Equal(facultyViewModel.Tags, faculty.FacultyTags.Select(x => x.Tag));
}

This works fine when I run it on its own, but when I run a second test (not shown) in parallel for a different mapping (for unrelated classes) that also uses a TypeAdapterConfig call, one or the other fails randomly every time. In each case, only the fields which are mapped with TypeAdapterConfig fail---in this case, the NotNull assertion fails.

I stepped through it several times in the Resharper test runner and I can see that both TypeAdapterConfig calls are executed in parallel, but only one takes effect. This also happens with the XUnit test runner.

[malramsay]

We may have to pull Mapster from our project for the same reasons as centur. As we have written more tests, we are getting tests randomly fail more often on our build server when the tests are run concurrently.

The failure occurs with our custom mappings. Similar to centur, we set them up in a static method in a static class. We even implemented the following pattern to stop the mappings being fired more than once. It did not help.

If anyone has any ideas let me know!

public static class CustomMappings
{
    private static bool _isConfigured;

    public static void Create()
    {
        if (_isConfigured)
            return;

        lock (typeof(CustomMappings))
        {
            if (!_isConfigured)
            {
                ConfigureAllData();
                _isConfigured = true;
            }
        }
    }

    private static void ConfigureAllData()
    {
        // custom mappings here
    }
}

[chaowlert]

@malramsay, your solution should work in theory. Do you call .Compile() inside ConfigureAllData in each config to eagerly compile? Pls also print error stack trace to get idea where is the issue.

[malramsay]

Actually, I do not call Compile() on each config to eagerly compile. I'm guessing I'm supposed to? This does not seem obvious from your examples on the wiki. I just had another quick look and couldn't find the Compile() method on there. I'd like to politely suggest you highlight it further on the wiki.

[dkempkes]

@mikebridge I was facing the same issue.

Adding a file

xunit.runner.json

with

{
  "maxParallelThreads": 1
}

solved my issue

https://xunit.github.io/docs/configuring-with-json.html

[chaowlert]

Closing this, as there are workaround solution.

[42ama]

The solution can still work in the context of parallel execution, if you wrap config setting and compilation in lock. A more efficient solution would be to move the mapping configuration out of the start of each test to a single moment of configuration before all tests are run, but this is not related to the framework used.

[DocSvartz]

Hi @42ama

There are at least two fundamental problems here:

1. In its current form, config setting is essentially an "endless configuration" and In this regard, the process itself cannot be made "atomic" using lock or other synchronization primitives;
2. Adapt (compilation) can also be called at any time, or not at all.

Example :

// thread 1: 

TypeAdapterConfig<SimplePoco, WeirdPoco>.NewConfig()
                               .Map(dest => dest.IHaveADifferentId, src => src.Id)
                               .Map(dest => dest.MyNamePropertyIsDifferent, src => src.Name)
                               .Ignore(dest => dest.Children); 

// thread 2: 

 TypeAdapter.Adapt<WeirdPoco>();

I think that in fact the thread requesting Adapt can get control before any setup statement (.Map or .Ignore etc.) is executed.
(In the given example, this usually happens before calling Ignore).

As a result:

1. You end up with an incomplete configuration.
2. You cannot further configure it, since the configuration has already been compiled and cannot be modified.

If you are interested in this topic, you can join the discussion here.