Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UmbracoMapper InvalidOperationException due to concurrent modification of inner dictionary #15849

Merged
merged 1 commit into from Mar 14, 2024

Conversation

albinj
Copy link
Contributor

@albinj albinj commented Mar 7, 2024

Prerequisites

  • I have added steps to test this contribution in the description below

Description

Fixes issue #15830 by changing the inner Dictionary of _ctors to a ConcurrentDictionary.

This concurrency issue was encountered when mapping one type of object to multiple other objects (multiple mapping definitions) and performing multiple concurrent requests when Umbraco boots to a url with a view that did all the mappings at once.

Error: Umbraco.Cms.Web.Common.PublishedModels.FrontPage => Site.MyModel
System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
at Umbraco.Cms.Core.Mapping.UmbracoMapper.GetCtor(Type sourceType, Type targetType)
at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, Type sourceType, MapperContext context)
at Umbraco.Cms.Core.Mapping.UmbracoMapper.Map[TTarget](Object source, MapperContext context)
at Umbraco.Cms.Core.Mapping.MapperContext.Map[TTarget](Object source)

The stack trace points to the UmbracoMapper.GetCtor method as the origin of the exception, indicating that the issue occurs during the retrieval of constructor mappings in concurrent scenarios.

Steps to test

  • Implement an UmbracoMapper mapping, for example, mapping from Umbraco.Cms.Web.Common.PublishedModels.FrontPage to Site.MyModel.
  • Use the mapper in a razor view
  • Simultaneously send multiple requests during Umbraco startup to the view rendering the mapped models using tools like curl to simulate concurrent access.
  • Monitor the application for any instances of InvalidOperationException indicating corrupted state due to concurrent modifications.
  • If it does not work, restart Umbraco and try again.
  • This PR fixes the InvalidOperationException from UmbracoMapper

Copy link

github-actions bot commented Mar 7, 2024

Hi there @albinj, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

  • It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
  • The automated tests all pass (see "Checks" tab on this PR)
  • The level of security for this contribution is the same or improved
  • The level of performance for this contribution is the same or improved
  • Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
  • If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
  • 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@albinj albinj changed the title Fix UmbracoMapper InvalidOperationException due to concurrent modification of inner dictionary (#15830) Fix UmbracoMapper InvalidOperationException due to concurrent modification of inner dictionary Mar 7, 2024
@georgebid
Copy link
Contributor

Hey @albinj, thanks a lot for your PR to fix #15830! Someone on the core collaborators team will review this soon 😄

@nul800sebastiaan nul800sebastiaan merged commit 4a977b8 into umbraco:contrib Mar 14, 2024
14 checks passed
@nul800sebastiaan
Copy link
Member

Thanks @albinj - looks good to me, thanks for the assist! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants