8000 CSHARP-5894: Prevent deadlock during multi-theaded BsonClassMap serializer resolution by damieng · Pull Request #1890 · mongodb/mongo-csharp-driver · GitHub
[go: up one dir, main page]
Skip to content

CSHARP-5894: Prevent deadlock during multi-theaded BsonClassMap serializer resolution#1890

Open
damieng wants to merge 2 commits intomongodb:mainfrom
damieng:csharp5894
Open

CSHARP-5894: Prevent deadlock during multi-theaded BsonClassMap serializer resolution#1890
damieng wants to merge 2 commits intomongodb:mainfrom
damieng:csharp5894

Conversation

@damieng
Copy link
Contributor
@damieng damieng commented Feb 25, 2026

Fixes CSHARP-5894

v3.x introduces a deadlock when deadlock occurs when two threads concurrently call BsonClassMap.LookupClassMap for related types involving two independent locks:

  • Thread A: holds ConfigLock WRITE (inside Freeze) -> accesses a Lazy<IBsonSerializer>.Value on a cached DictionarySerializer -> blocks on the Lazy's internal lock
  • Thread B: holds that Lazy's internal lock (factory still executing) -> needs ConfigLock WRITE to complete LookupClassMap for a nested type -> blocks

The shared Lazy instance comes from the serializer registry's ConcurrentDictionary cache - both threads resolve the same Dictionary<string, T> type and get back the same serializer object with the same Lazy fields.

Regression from v2.x

This was not happening in 2.x as AutoMap() ran inside the ConfigLock WRITE lock which forced all resolution onto the same thread.

https://github.com/mongodb/mongo-csharp-driver/blob/v2.x/src/MongoDB.Bson/Serialization/BsonClassMap.cs#L350

BsonSerializer.ConfigLock.EnterWriteLock();
try
{
    classMap.AutoMap();       // under WRITE lock
    RegisterClassMap(classMap);
    return classMap.Freeze();
}

ConfigLock is a ReaderWriterLockSlim with SupportsRecursion, which allows recursive lock acquisition on the same thread to succeed without contention. No second thread could interleave because it would be blocked on ConfigLock.

v3.x moved AutoMap() outside the lock to solve a [different deadlock problem[(https://github.com//pull/1436/files).

// current
newClassMap.AutoMap();            // no lock held
BsonSerializer.ConfigLock.EnterWriteLock();
try
{
    RegisterClassMap(newClassMap);
    return classMap.Freeze();
}

This allows threads to do expensive AutoMap work in parallel, but it opened a window where Thread B can be mid-AutoMap (holding a Lazy internal lock, no ConfigLock) while Thread A acquires ConfigLock and reaches the same Lazy through a different path (Freeze -> LookupClassMap(baseType) -> AutoMap -> same cached serializer -> same Lazy).

The fix

Changing Lazy instances on serializers whose factories call serializerRegistry.GetSerializer to use LazyThreadSafetyMode.PublicationOnly instead of the default ExecutionAndPublication.

The default mode acquires an internal lock so that only one thread executes the factory -- all other threads block on .Value until the first thread completes. This blocking is what creates the deadlock cycle.

PublicationOnly removes that internal lock entirely. Multiple threads can execute the factory concurrently, and the first result to complete is published as the value. The rest are discarded. This is safe here because serializerRegistry.GetSerializer is idempotent and the serializer instances are functionally equivalent.

Lazy instances wrapping already-resolved serializers (e.g. new Lazy<T>(() => itemSerializer)) are unchanged -- their factories return immediately and can never participate in a lock cycle.

This is achieved with a new "Lazy" helper that not only provides the PublicationOnly but uses the generic method resolution to avoid having to specify the type args to "new".

A test to cover it is included but it does not cover ALL possible factory resolvers.

Copilot AI review requested due to automatic review settings February 25, 2026 22:09
@damieng damieng requested a review from a team as a code owner February 25, 2026 22:09
@damieng damieng requested a review from ajcvickers February 25, 2026 22:09
Copilot AI reviewed Feb 25, 2026
View reviewed changes
Copy link
Contributor
Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Addresses a deadlock regression in v3.x where concurrent BsonClassMap.LookupClassMap calls can deadlock during nested serializer/class map resolution by removing blocking Lazy<T>.Value contention.

Changes:

  • Switches several serializer-internal Lazy<IBsonSerializer<...>> fields (whose factories call serializerRegistry.GetSerializer(...)) to LazyThreadSafetyMode.PublicationOnly.
  • Introduces an internal Lazy.CreatePublicationOnly helper to standardize creating PublicationOnly lazies.
  • Adds a concurrency regression test intended to reproduce/guard against the deadlock scenario.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/MongoDB.Bson.Tests/Serialization/BsonClassMapConcurrencyTests.cs Adds a targeted concurrency regression test for the deadlock scenario.
src/MongoDB.Bson/Serialization/Serializers/Lazy.cs Adds helper factory for Lazy<T> configured as PublicationOnly.
src/MongoDB.Bson/Serialization/Serializers/TupleSerializers.cs Uses PublicationOnly for tuple item serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/ValueTupleSerializers.cs Uses PublicationOnly for valuetuple item serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/DictionarySerializerBase.cs Uses PublicationOnly for key/value serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/KeyValuePairSerializer.cs Uses PublicationOnly for key/value serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/EnumerableSerializerBase.cs Uses PublicationOnly for item serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/IEnumerableDeserializingAsCollectionSerializer.cs Uses PublicationOnly for item serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/NullableSerializer.cs Uses PublicationOnly for underlying serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/ImpliedImplementationInterfaceSerializer.cs Uses PublicationOnly for implementation serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/TwoDimensionalArraySerializer.cs Uses PublicationOnly for item serializer lazy resolution (both provided serializer and registry paths).
src/MongoDB.Bson/Serialization/Serializers/ThreeDimensionalArraySerializer.cs Uses PublicationOnly for item serializer lazy resolution via registry.
src/MongoDB.Bson/Serialization/Serializers/SerializeAsNominalTypeSerializer.cs Uses PublicationOnly for nominal serializer lazy resolution (both provided serializer and registry paths).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@BorisDog BorisDog requested a review from sanych-sun February 25, 2026 22:55
@BorisDog BorisDog removed the request for review from ajcvickers February 25, 2026 23:48
@damieng damieng added the bug Fixes issues or unintended behavior. label Mar 2, 2026
@damieng damieng changed the title CSHARP-5894: Preent deadlock during multi-theaded BsonClassMap serializer resolution CSHARP-5894: Prevent deadlock during multi-theaded BsonClassMap serializer resolution Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sanych-sun sanych-sun Awaiting requested review from sanych-sun

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

bug Fixes issues or unintended behavior.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@damieng
0