[SERVER-35619] Potential deadlock in ShardRegistry::replicaSetChangeShardRegistryUpdateHook Created: 15/Jun/18  Updated: 08/Jan/24  Resolved: 09/Mar/20

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: 4.7.0

Type: Bug Priority: Major - P3
Reporter: Mathias Stearn Assignee: Randolph Tan
Resolution: Fixed Votes: 0
Labels: neweng, sharding-wfbf-day
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Sharding 2018-07-16, Sharding 2018-07-30, Sharding 2018-08-13, Sharding 2018-09-24, Sharding 2020-03-09, Sharding 2020-03-23
Participants:

 Description   

ShardRegistry::replicaSetChangeShardRegistryUpdateHook is registered in ReplicaSetMonitor::setSynchronousConfigChangeHook() which is explicitly documented as calling the hook while holding the RSM mutex. Unfortunately, that hook acquires ShardRegistyData::_mutex at https://github.com/mongodb/mongo/blob/82b62cf1e513657a0c35d757cf37eab0231ebc9b/src/mongo/s/client/shard_registry.cpp#L526.

 

These mutexes are acquired in the other order in ShardRegistryData::toBSON() when it transitively calls into ReplicaSetMonitor::getServerAddress(), so this could result in a deadlock.

Suggested Fix

1. Split off the serverAddressLock in ReplicaSetMonitor so the lock that is eventually taken from ShardRegistry is not the same as taken in Refresher::_refreshUntilMatches
2. Build the confirmed server address into a separate variable and update it when the seedNodes set in RSM is being updated as the confirmedServerAddress is built from seedNodes nodes.



 Comments   
Comment by Githook User [ 09/Mar/20 ]

Author:

{'username': 'renctan', 'name': 'Randolph Tan', 'email': 'randolph@10gen.com'}

Message: SERVER-35619 Do not hold ShardRegistryData::_mutex while trying to take ScanningReplicaSetMonitor::SetState::mutex
Branch: master
https://github.com/mongodb/mongo/commit/afad879071031c61013e3f58248adcc9f9f5d1c6

Comment by Sheeri Cabral (Inactive) [ 06/Feb/20 ]

Kal's simpler suggestion was agreed upon by Kal, Jason and Esha in a triage meeting today.

Comment by Kaloian Manassiev [ 11/Feb/19 ]

As mentioned in the description, this locking order violation only happens under ShardRegistry::toBSON (which is only used by the getShardMap command), this is fairly unlikely to hit in practice, but if it did happen it will stall the entire node.

Could a potential simpler fix be to just copy the shard names here into a string vector, then release the ShardRegistryData::_mutex and use getShard outside of the lock in order to obtain the shard's connection strings and other data.

This could potentially not return all shards if the command is run right when shard membership changes, but so be it - this is an extremely rare condition to warrant a complex solution involving introducing a third mutex.

Generated at Thu Feb 08 04:40:25 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.