Mapster icon indicating copy to clipboard operation
Mapster copied to clipboard

Fix Race Condition between Configuration and Adapt()

Open DocSvartz opened this issue 2 months ago • 2 comments

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 avatar Nov 06 '25 07:11 DocSvartz

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

DocSvartz avatar Nov 06 '25 07:11 DocSvartz

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 avatar Nov 24 '25 06:11 DocSvartz