Automapper: InvalidOperationException : ValueFactory attempted to access the Value property of this instance.

Created on 9 Dec 2016  ·  27Comments  ·  Source: AutoMapper/AutoMapper

This issue was introduced to us after updating to version 5.2.0.

We have a test helper which creates our mapper and adds profiles for us via reflection (we also tried to wrap it with a lock).

private static IMapper MapAutoMapper()
{
    lock (new object())
    {
        var profiles = from assembly in AppDomain.CurrentDomain.GetAssemblies().Where(x => x.FullName.StartsWith("ProjectName."))
                       from profileType in assembly.GetExportedTypes()
                       where typeof(Profile).IsAssignableFrom(profileType) && !profileType.GetTypeInfo().IsAbstract
                       select (Profile)Activator.CreateInstance(profileType);

        var config = new MapperConfiguration(cfg =>
        {
            foreach (var profile in profiles)
            {
                cfg.AddProfile(profile);
            }
        });

        return config.CreateMapper();
    }
}

This often throws the exception down below on our build server (TeamCity). This should be related to a multi-threading issue as Lazy<> is involved which is not thread-safe, right?

System.InvalidOperationException : ValueFactory attempted to access the Value property of this instance.
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.LazyInitValue()
   at AutoMapper.Mappers.ConvertMapper.MapExpression(TypeMapRegistry typeMapRegistry, IConfigurationProvider configurationProvider, PropertyMap propertyMap, Expression sourceExpression, Expression destExpression, Expression contextExpression)
   at AutoMapper.Execution.TypeMapPlanBuilder.MapExpression(TypeMapRegistry typeMapRegistry, IConfigurationProvider configurationProvider, TypePair typePair, Expression sourceParameter, Expression contextParameter, PropertyMap propertyMap, Expression destinationParameter)
   at AutoMapper.Execution.TypeMapPlanBuilder.CreatePropertyMapFunc(PropertyMap propertyMap)
   at AutoMapper.Execution.TypeMapPlanBuilder.TryPropertyMap(PropertyMap propertyMap)
   at AutoMapper.Execution.TypeMapPlanBuilder.CreateAssignmentFunc(Expression destinationFunc, Boolean constructorMapping)
   at AutoMapper.Execution.TypeMapPlanBuilder.CreateMapperLambda()
   at AutoMapper.TypeMap.Seal(TypeMapRegistry typeMapRegistry, IConfigurationProvider configurationProvider)
   at AutoMapper.MapperConfiguration.Seal()
   at AutoMapper.MapperConfiguration..ctor(MapperConfigurationExpression configurationExpression, IEnumerable`1 mappers)
   at AutoMapper.MapperConfiguration..ctor(Action`1 configure)
   at Shared.TestHelper.WithFakes.MapAutoMapper() in C:\Something\Something\Project\Path\WithFakes.cs:line xx

Any ideas?

Bug

Most helpful comment

We've got a pending PR for this one.

On Thu, Dec 15, 2016 at 9:58 PM, Cindy notifications@github.com wrote:

I am having the same error. It only happens when I try to run all our
tests at once. I create a new mapper in each test class, but if all of the
tests are running at the same time, then we have multiple threads, each
creating a mapper in the test class, so we get this collision. This didn't
happen before upgrading to 5.2.0 - we were able to run all of them at one
time just fine. Each of our test classes has code in the constructor that
looks like this:

` // create the mapping configuration
var config = new MapperConfiguration
(
map =>
{
map.AddProfile();
map.AddProfile();
});

        // assert that the configuration is valid
        config.AssertConfigurationIsValid();

        this.mapper = config.CreateMapper();

`

We are quite dependent on AutoMapper in our application, and having it not
work in test or on a Build Server when it runs all of the tests for an
application at one time, is not really acceptable.

What is the correct way to solve this problem?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/AutoMapper/AutoMapper/issues/1847#issuecomment-267511730,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMgDcZwtagZxJpo8Wffnq_3gUb5Nhks5rIgx2gaJpZM4LJCMf
.

All 27 comments

Initialization should happen on a single thread, before any other use of AM. Only after the init phase is done, you're free to share the resulting mapper object. So if you have a repro in these conditions, I want to see it :)

It might make sense if you're not on 5.2.0.

Sorry, I forgot to mention that this code ran just fine before version 5.2.0. What has changed since then that it worked perfectly fine before? If this is the only way... then we have to think about how to handle this... Guessing that TeamCity kicks off several threads for running the xUnit tests.

The static Mapper seems more appropriate for this case. But you still have to call Initialize only once.

And another thing, lock(new object()) doesn't lock anything. Object identity is key here. But this code doesn't look good regardless.

Yeah you're right regarding the lock object.

What's wrong with the code? Simple approach. Look for profiles, add them, create an instance.

The question still remains regarding what has changed under the covers that it doesn't work anymore with 5.2.0.

Why the init code runs more than once? That's unexpected.

We did a lot of config changes underneath the covers.

But I'd suggest you use Lazy and not this locking. It's smarter than you
or I about this sort of thing

On Fri, Dec 9, 2016 at 9:44 AM, timmkrause notifications@github.com wrote:

Yeah you're right.

What's wrong with the code? Simple approach. Look for profiles, add them,
create an instance.

The question still remains regarding what has changed under the covers
that it doesn't work anymore with 5.2.0.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/AutoMapper/AutoMapper/issues/1847#issuecomment-266045050,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMjpqEHvEfRctjfX0Ih5HJfuuRR2Zks5rGXdQgaJpZM4LJCMf
.

In all fairness, no locks are needed here, because there is no shared state. The shared state is in AM, see #1848. But when you do smth unexpected, things happen :)

Hmm the exception message says the issue is thrown in ConvertMapper which looks like an AutoMapper bug. Looks like ConvertMapper isn't thread safe when resolving multiple instances of IMapper at the same time is my guess.

No init code in AM should be thread safe. The instances were shared because they were placed in statics.

We have done it with Lazy<> before version 5.2.0. Since 5.2.0 we have this issue even with Lazy.
Then we tried it without lazy. And THEN we tried it with this ugly lock.

Before 5.2.0:

private static readonly Lazy<IMapper> AutoMapper = new Lazy<IMapper>(MapAutoMapper);

But this would mean it happens all the time, even when you init once, which is clearly a bug. But it might depend on your particular setup. So a repro would help.

I say that because I see the error when calling init more than once on different threads, but I don't see it under a lock, let alone calling once.

Sorry, currently we don't have the time for a repro. We're downgrading to 5.0.2 again.
Others seem to have faced a problem with Lazy as well: https://github.com/elmah/Bootstrapper/issues/5

Maybe this helps?

I will try to build a repro next week if the provided link doesn't help.

Hmm, no :) We only fix issues we can see for ourselves. I don't think it's possible for that error to appear unless you call init code concurrently.

And this happens... We have this WithFakes class from which test classes inherit, so in case of multiple threads executing unit tests this will happen I would assume.

Well, yes, but this should be worked around with a lock, or even better by calling init once. Above you said that it fails also when calling init once with Lazy and that seems impossible.

The docs say that even Lazy is not thread safe and shouldn't be initialized more than once by multiple threads...

"The Lazy instance is not thread safe; if the instance is accessed from multiple threads, its behavior is undefined. Use this mode only when high performance is crucial and the Lazy instance is guaranteed never to be initialized from more than one thread. If you use a Lazy constructor that specifies an initialization method (valueFactory parameter), and if that initialization method throws an exception (or fails to handle an exception) the first time you call the Lazy.Value property, then the exception is cached and thrown again on subsequent calls to the Lazy.Value property. If you use a Lazy constructor that does not specify an initialization method, exceptions that are thrown by the default constructor for T are not cached. In that case, a subsequent call to the Lazy.Value property might successfully initialize the Lazy instance. If the initialization method recursively accesses the Value property of the Lazy instance, an InvalidOperationException is thrown."

https://msdn.microsoft.com/de-de/library/system.threading.lazythreadsafetymode(v=vs.110).aspx

Maybe it is as easy as using LazyThreadSafetyMode.ExecutionAndPublication?

"Locks are used to ensure that only a single thread can initialize a Lazy instance in a thread-safe manner."

By default it's thread safe.

Same error here, and 5.1.1 work fine. Will investigate further

I am having the same error. It only happens when I try to run all our tests at once. I create a new mapper in each test class, but if all of the tests are running at the same time, then we have multiple threads, each creating a mapper in the test class, so we get this collision. This didn't happen before upgrading to 5.2.0 - we were able to run all of them at one time just fine. Each of our test classes has code in the constructor that looks like this:

                // create the mapping configuration
                var config = new MapperConfiguration
                    (
                    map =>
                    {
                        map.AddProfile<ABCProfile>();
                        map.AddProfile<XYZProfile>();
                    });

                // assert that the configuration is valid
                config.AssertConfigurationIsValid();

                this.mapper = config.CreateMapper();

We are quite dependent on AutoMapper in our application, and having it not work in Test or on a Build Server when it runs all of the tests for an application at one time, is not really acceptable.

What is the correct way to solve this problem?

We've got a pending PR for this one.

On Thu, Dec 15, 2016 at 9:58 PM, Cindy notifications@github.com wrote:

I am having the same error. It only happens when I try to run all our
tests at once. I create a new mapper in each test class, but if all of the
tests are running at the same time, then we have multiple threads, each
creating a mapper in the test class, so we get this collision. This didn't
happen before upgrading to 5.2.0 - we were able to run all of them at one
time just fine. Each of our test classes has code in the constructor that
looks like this:

` // create the mapping configuration
var config = new MapperConfiguration
(
map =>
{
map.AddProfile();
map.AddProfile();
});

        // assert that the configuration is valid
        config.AssertConfigurationIsValid();

        this.mapper = config.CreateMapper();

`

We are quite dependent on AutoMapper in our application, and having it not
work in test or on a Build Server when it runs all of the tests for an
application at one time, is not really acceptable.

What is the correct way to solve this problem?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/AutoMapper/AutoMapper/issues/1847#issuecomment-267511730,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMgDcZwtagZxJpo8Wffnq_3gUb5Nhks5rIgx2gaJpZM4LJCMf
.

Try the MyGet build.

@lbargaoanu, @jbogard: Successfully tested this on TeamCity with MyGet package version 5.3.0-alpha-01276 today. Ran 3 builds without errors. Thank you so much! Looking forward to the next version! 👍

Updating to the latest stable release 6.0.2 fixed my solution. Just wanted to say great work.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jepl4052 picture jepl4052  ·  6Comments

vivainio picture vivainio  ·  6Comments

WellspringCS picture WellspringCS  ·  5Comments

zolakt picture zolakt  ·  4Comments

Mds92 picture Mds92  ·  5Comments