Random null-ref exceptions in cache implementation #212
Comments
|
Thanks for letting me know - It's a funny place for it to fall over! - I'll check it out soon. |
|
here's another one, different stack trace but also a null-ref: |
|
@SteveWilkes you might want to look into that ReSharper suppression in the key comparer implementation: AgileMapper/AgileMapper/Caching/DefaultComparer.cs Lines 3 to 12 in 6f51655 I assume the suppression is there due to some sort of guarantee that If |
|
Thanks both! I'll look at getting a bug-fix release sorted out over the weekend. |
|
@SteveWilkes , over the weekend I realized exactly what is causing these problems. The root of the issue lies here: var lazyAgileMapper = new Lazy<AgileObjects.AgileMapper.IMapper>(() => AgileObjects.AgileMapper.Mapper.CreateNew());
container.Configure(c => c.For<AgileObjects.AgileMapper.IMapper>().Use(() => lazyAgileMapper.Value));Notice how we lazily initialize the mapper, then register it in the container with a factory argument. This is seemingly ok, if it wasn't for the fact that registrations are transient by default. What happens here is that the container (StructureMap in this case) takes "ownership" of the created instance and disposes it after each request. However, it is actually disposing a shared instance, that could be in use by a different thread serving a different request. This clearly means the following:
You should be able to repro the issue on your side by introducing some artificial delays in the cache access code while a mapping is taking place, and then, in parallel, dispose of the mapper instance. While you can probably fix this problem by making your cache thread-safe, I also suggest another change: throw I believe that, if such exception was present, it wouldn't've taken this long for us to realize our mistake in the registration code and consequently the impact of the problem would've been reduced significantly. |
|
Hi @julealgon - that's a great find, thanks very much! I'm in the process of putting together a v1.8 release - I'll add the Disposed checks as you suggest. |
|
The v1.8 branch now handles null keys in the default cache key comparer, throws Thanks again! |
SteveWilkes
added
bug
in-branch
|
Hi, I've now uploaded a preview v1.8 release to NuGet with the caching and ObjectDisposed issues fixed - please could you try it out and let me know how you get on? Thanks, Steve |
SteveWilkes
added
in-preview
|
Have you had a chance to try the v1.8 preview, @replaysMike ? |
* Updating AO packages * Adding .NET Standard 2.0 target / Rationalising compiler directives / Removing non-latest-major-version .NET Core test projects * Fixing caching thread safety issue / Handling null keys in default key comparer, re: #212 * Throwing ObjectDisposedException on attempt to use a disposed mapper * Simplifying numeric constants setup, re: #213 * Tidying * Improving target member selection, re: #209 * Adding v1.8 preview NuGet package * Tidying documentation * Organising classes * Features/target member matcher data sources (#214) * Adding data source -> target member matcher selection * Member matcher data sources / Conflict testing / Documentation improvements * Fixing query projection * All fixed * Adding documentation * Updating release notes / Updating to v1.8 * Extra test coverage * Adding v1.8 NuGet package
SteveWilkes
removed
the
in-preview
|
The changes made for this issue are in the full v1.8 release, which is now available on NuGet. Thanks! |
We are seeing random failures in our build pipeline which are caused by a bug within AgileMapper. We aren't using the static mapper, but rather
Mapper.CreateNew()which seems to be causing the problem. We don't appear to have the issue when using the static mapper instance. It seems to be a thread safety issue but I'm not familiar enough with your implementation to narrow it down further.The text was updated successfully, but these errors were encountered: