Skip to content

Conversation

@DocSvartz
Copy link

@DocSvartz DocSvartz commented Nov 6, 2025

Based on these tests and #86, #138

Depending on the speed and order of execution (configuration and Adapt()), one of the following situations can occur:

  1. Incomplete configuration (the main cause of errors in the tests mentioned at the beginning);
  2. "Incomplete cleanup" - Exception: "TypeAdapter.Adapt was already called, please clone or create a new TypeAdapterConfig."
  3. Exception to collection modification during enumeration.

Additional problem:
The current configuration design does not provide information about configuration completion.

add This PR

  1. Thread safety creating a new configuration:
 TypeAdapterConfigConcurrency<WhenAddingCustomMappings.SimplePoco, WeirdPoco>
                .NewConfig(cfg =>
                {
                    cfg
                        .Map(dest => dest.IHaveADifferentId, src => src.Id)
                        .Map(dest => dest.IHaveADifferentId, src => src.Id)
                        .Map(dest => dest.MyNamePropertyIsDifferent, src => src.Name)
                        .Ignore(dest => dest.Children);
                });

  1. Thread safety loading configurations:

TypeAdapterConfig.GlobalSettings.ScanConcurrency(Assembly.GetExecutingAssembly());

@DocSvartz DocSvartz requested review from andrerav and stagep November 6, 2025 07:22
@DocSvartz
Copy link
Author

@andrerav @stagep See if this is necessary at all?
Actually, I don't fully understand the meaning of the initial tests :)

@DocSvartz
Copy link
Author

This problem can perhaps be partially solved using Freezing config (without throwing exceptions when changing). However, this needs to be tested with at least 1 million iterations.

But maybe I'm not quite understanding the real world use case for this.

The only real use case I see is when different adapt configurations are set in different threads.
But in that case, the Adapt call itself must be called on the same thread. (This should be a single lock, covering both the configuration and the Adapt call.)

@DocSvartz DocSvartz marked this pull request as draft November 28, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant