34

I'm looking up AutoMapper code now (evaluating it for one of projects I'm working on), and, frankly speaking, I'm quite surprised:

  • The library API is based on a single static access point (Mapper type), so generally any of its methods must be thread safe
  • But I didn't find ANY evidence of this in code.

All I was able to find is this issue, but even the statement made there seems incorrect: if Map doesn't use thread-safe data structures internally, it can't be considered as thread-safe as well, if I'm going to call CreateMap in non-concurrent context, but concurrently with Map.

I.e. the only possible usage pattern of AutoMapper in e.g. ASP.NET MVC application is:

lock (mapperLock) {
    ... Mapper.AnyMethod(...) ...
}

Obviously, if I'm correct, that's a huge lack.

So I have two questions:

  • Am I correct?
  • If yes, what's the best alternative to AutoMapper that doesn't have this issue?
Alex Yakunin
  • 6,330
  • 3
  • 33
  • 52
  • The main key seems to be the double-checked lookup via `ThreadSafeList _typeMaps`; what makes you think it is not thread-safe? What ***specifically*** do you believe is not thread-safe? – Marc Gravell May 18 '12 at 08:36
  • Is TypeMap an immutable object? – Alex Yakunin May 18 '12 at 08:40
  • you tell me! (and the questions is also: even if it isn't, is it inappropriately updated at any point, other than by you). You've made a claim that it is not thread-safe; please elaborate on what you think is not safe. Note that typically the strategy (once built) is not updated, so the only thing that needs protecting is access to the strategy-cache, which appears to be done correctly. – Marc Gravell May 18 '12 at 08:42
  • Looking up the code now once more. Actually, I missed usage of `ThreadSafeList` there. – Alex Yakunin May 18 '12 at 08:47
  • Am I correct that `TypeMap` entry is added right on `Mapper.CreateMap` call, and further modified by `IMappingExpression` methods? If yes, there is an issue, since concurrent thread may access inconsistent `TypeMap` while it is modified by a thread invoking `CreateMap`. – Alex Yakunin May 18 '12 at 08:50
  • re CreateMap: isn't it only added to the list at the *end* of construction? – Marc Gravell May 18 '12 at 08:52
  • 2
    I think this question is totally valid. We cannot assume the library to be thread-safe because we haven't found evidence against it. It is the other way around: everything is unsafe until proven otherwise. I guess the author of this lib needs to step in. – usr May 18 '12 at 08:53
  • To fully explain the case: I'm asking this because I'd like to configure automatter in-place, i.e. right before usage. I planned to configure it in non-concurrent context, i.e. ~ `lock (mapperConfigLock) { Mapper.CreateMap()....; }`, and I fear this is not enough now. – Alex Yakunin May 18 '12 at 08:55
  • See e.g. this test code: https://github.com/AutoMapper/AutoMapper/blob/master/src/UnitTests/CustomMapping.cs - I can't imagine how `TypeMap` there can be added after `CreateMap` call. I suspect the entry is added right on this call, and modified by `ForMember`-like methods further. – Alex Yakunin May 18 '12 at 08:57
  • Ok, an exact code from Automapper: `public void ConvertUsing(Type typeConverterType) { Type type = ...; this._typeMap.UseCustomMapper ...; }` – Alex Yakunin May 18 '12 at 09:01
  • One more example: `private void ForDestinationMember(...) { this._propertyMap = this._typeMap.FindOrCreatePropertyMapFor(destinationProperty); ...` – Alex Yakunin May 18 '12 at 09:07
  • Concerning thread safety of Automapper: IMO, the question is important. It's one of the most watched projects on C# on GitHub with all the consequences. – Alex Yakunin May 18 '12 at 09:12
  • This is one of the reasons I'm looking at moving to clearly instantiated configuration - to make these kinds of things "obvious". – Jimmy Bogard Jul 15 '12 at 22:20
  • Will be fixed in 3.2.0 https://github.com/AutoMapper/AutoMapper/issues/473 – hansmaad Mar 27 '14 at 08:02

2 Answers2

37

The linked issue more or less answers your questions:

Mapper.CreateMap is not threadsafe, nor will it ever be. However, Mapper.Map is thread-safe. The Mapper static class is just a thin wrapper on top of the MappingEngine and Configuration objects.

So only use Mapper.CreateMap if you do your configuration in one central place in a threadsafe manner.

Your comment was:

I'm asking this because I'd like to configure automatter in-place, i.e. right before usage. I planned to configure it in non-concurrent context, i.e. ~ lock (mapperConfigLock) { Mapper.CreateMap()....; }, and I fear this is not enough now.

If you are doing in-place configuration just don't use the static Mapper class. As the comment on the github issue suggest use the mapping engine directly:

var config = 
    new ConfigurationStore(new TypeMapFactory(), MapperRegistry.AllMappers());
config.CreateMap<Source, Destination>();
var engine = new MappingEngine(config);

var source = new Source();
var dest = engine.Map(source);

It's a little bit of more code but you can create your own helpers around it. But everything is local in a given method so no shared state no need to worry about thread safety.

nemesv
  • 138,284
  • 16
  • 416
  • 359
  • 'one central place in a threadsafe manner' - I think this anyway needs to be explained in the issue, since there is a plenty of room for possible mistakes. – Alex Yakunin May 18 '12 at 10:35
  • 2
    Here is link to mentioned [github issue](https://github.com/AutoMapper/AutoMapper/issues/514). – Aleksei Nov 04 '15 at 12:30
0

This questions may be a little bit outdated, just want to record some of my findings after a little bit of investigation.

Mapper is a wrapper class to wrap to create new configuration, and new instance of mapper inside static memory, so strictly speaking it is not thread-safe, but you can use it safe as long as you only initialize the configuration once.

MapperConfiguration create new instance of mapper, and record the config inside its own instance memory space.

TLDR;

If you need to init the configuration ONCE only, choose the static API

If you need to init the configuration many times, and worry about the the thread safety, choose instance API

code4j
  • 4,208
  • 5
  • 34
  • 51