I think this is still wrong... there just isn't a test to show that it is.

Consider this scenario: there might be a registered discriminator convention at this level and we're not looking it up.

Calling GetOrRegisterDiscriminatorConvention instead of RegsiterDiscriminatorConvention is in order to avoid collisions between threads.

How does it avoid collisions?

The new GetOrRegisterDiscriminatorConvention method uses the same lock as RegsiterDiscriminatorConvention to handle thread safety.

These don't need to be public.

Is this whole block really needed? Seems a bit redundant to check for a registered convention here and then check again later on below when trying to register.

It may seem redundant but it's not. All the other methods use the same pattern.

The idea is that after the initial configuration phase is over everything can be done with read lock only, eliminating lock contention between threads.

The second check below is because of multithreading. Between releasing the read lock and acquiring the write lock some other thread might have snuck in and registered the convention.

Does GetOrRegisterDiscriminatorConvention returns the value we are looking for? Can we return it straight away without additional lookup below?

Calling LookupDiscriminatorConvention below has some side effects that we don't want to skip.

The main side effect is that it registers the convention at all levels between the _classType and where the discriminator convention is found.

Minor: this else block if unnecessary as there is return in true path of the check. Can simplify it to reduce the nesting level.

True... but at the same time the two conditions are mutually exclusive and the else makes that clearer in some ways.

Nevertheless changed it as requested.

I would suggest to have slightly different method signature to mimic ConcurrentDictionary.GetOrAdd method signature:

internal static IDiscriminatorConvention GetOrAddDiscriminatorConvention(Type type, Func<Type, IDiscriminatorConvention> discriminatorConventionFactory)

ConcurrentDictionary also has an overload like this one:

https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.getoradd?view=net-8.0#system-collections-concurrent-concurrentdictionary-2-getoradd(-0-1)

Since we don't need a factory I think this is OK as is.