8000 [RlsLB] Fix Deadlock by yashykt · Pull Request #37459 · grpc/grpc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RlsLB] Fix Deadlock #37459

Closed
wants to merge 9 commits into from
Closed

[RlsLB] Fix Deadlock #37459

wants to merge 9 commits into from

Conversation

yashykt
Copy link
Member
@yashykt yashykt commented Aug 12, 2024 •
edited
Loading

Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -

[mutex.cc : 1418] RAW: Potential Mutex deadlock:                                                         
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()                                                 
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()                                        
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()                        
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()                 
        @ 0x564f4c1f1216 std::default_delete<>::operator()()                                              
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()                                                  
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()                                                        
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()                                            
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()                                             
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()                                               
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()                           
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()                           
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()                                           
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()                                                 
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()                                     
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()            
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()            
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()                                           
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()                                         
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()                                             
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()                                     
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()          
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()          
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()                                           
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()                                         
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()                                             
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()                                     
        @ 0x564f4c26bafc std::pair<>::~pair()                                                             
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()                                          
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()                                             
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()                                               
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()                                                  
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()                                                      
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()                                                     
        @ 0x564f4c26a62a std::map<>::~map()                                                               
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()                                                                                                     
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()                                                                                                     
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()                                           
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
                                                                                                                                                                                                                   
[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed                                                          
[mutex.cc : 1432] RAW: Cycle:                       
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:                                                                                                                                                                    
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()                                    
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()                                               
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()            
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()         
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()                   
        @ 0x564f4c1f111b std::make_unique<>()                                                            
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()                                       
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()                                               
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()                     
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()                  
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()                                                                                                               
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()                                                          
        @ 0x564f4c2f80f6 std::__invoke_impl<>()                                                                                                                                                                    
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c s
8000
td::_Function_handler<>::_M_invoke()                                           
        @ 0x564f4b8ad682 std::function<>::operator()()                                                                                                                                                                                                                                                                                                                                                                                  
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()                                                                                     
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()                                                                                                                                                
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()                       
        @ 0x564f4cd75a72 exec_ctx_run()                                                                    
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()          
        @ 0x564f4cc8ee1d end_worker()  
        @ 0x564f4cc8f304 pollset_work()                                                                    
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()    
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck                                                     
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()                                                  
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject() 
                                                                                                           
[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:                                                         
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()                                    
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()                                               
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()  
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()                               
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()                  
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()                                                                                                    
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()                                                                                                                                            
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()                                                                                                   
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()                                                                                                          
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()                                                                                                                   
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()                                                                                                                                       
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()             
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()     
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()                    
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()                     
        @ 0x564f4bf1cd8b CivetServer::requestHandler()       
        @ 0x564f4bf35e7b handle_request                                                                    
        @ 0x564f4bf29534 handle_request_stat_log                                                           
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run      
        @ 0x564f4bf3a57f worker_thread         
        @ 0x7f93e9137ea7 start_thread     
                                                                                                           
[mutex.cc : 1454] RAW: dying due to potential deadlock                                                   
Aborted

From the stack, it looks like we are ending up holding a lock to the RlsLB policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is OpenTelemetry -> gRPC OpenTelemetry plugin -> gRPC Component like RLS/xDSClient.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in gRPC OpenTelemetry plugin. To avoid deadlocks, we remove the callback inside RlsLb from outside the critical region, but RlsLb owns refs to child policies which in turn hold refs to XdsClient. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in #37425.
Verified that it fixes the bug.

@yashykt yashykt changed the title Fix deadlocks [RlsLB] Fix Deadlock Aug 13, 2024
@yashykt yashykt marked this pull request as ready for review August 13, 2024 02:09
@yashykt yashykt requested a review from markdroth as a code owner August 13, 2024 02:09
@yashykt yashykt added the release notes: yes Indicates if PR needs to be in release notes label Aug 13, 2024
@yashykt
Copy link
Member Author
yashykt commented Aug 13, 2024

For reference, this is the stack trace with symbols.

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7a77859 in __GI_abort () at abort.c:79
#2  0x0000555556ecab1d in absl::lts_20240116::raw_log_internal::(anonymous namespace)::RawLogVA(absl::lts_20240116::LogSeverity, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (
    severity=absl::lts_20240116::LogSeverity::kFatal, file=0x55555748d20e "mutex.cc", line=1454, format=0x55555748da38 "dying due to potential deadlock", ap=0x7fffffffb998)
    at external/com_google_absl/absl/base/internal/raw_logging.cc:190
#3  0x0000555556ecaca4 in absl::lts_20240116::raw_log_internal::RawLog (severity=absl::lts_20240116::LogSeverity::kFatal, file=0x55555748d20e "mutex.cc", line=1454, 
    format=0x55555748da38 "dying due to potential deadlock") at external/com_google_absl/absl/base/internal/raw_logging.cc:252
#4  0x0000555556e5ff9b in absl::lts_20240116::DeadlockCheck (mu=0x555558035d40) at external/com_google_absl/absl/synchronization/mutex.cc:1454
#5  0x0000555556e6007a in absl::lts_20240116::DebugOnlyDeadlockCheck (mu=0x555558035d40) at external/com_google_absl/absl/synchronization/mutex.cc:1469
#6  0x0000555556e60382 in absl::lts_20240116::Mutex::Lock (this=0x555558035d40) at external/com_google_absl/absl/synchronization/mutex.cc:1526
#7  0x0000555555d24cfc in absl::lts_20240116::MutexLock::MutexLock (this=0x7fffffffbc18, mu=0x555558035d40) at external/com_google_absl/absl/synchronization/mutex.h:584
#8  0x0000555555d65679 in grpc::internal::OpenTelemetryPluginImpl::RemoveCallback (this=0x555558035cc0, callback=0x7fff8407c230) at external/com_github_grpc_grpc/src/cpp/ext/otel/otel_plugin.cc:830
#9  0x0000555556cf59bc in grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback (this=0x7fff8407c230, __in_chrg=<optimized out>) at external/com_github_grpc_grpc/src/core/telemetry/metrics.cc:99
#10 0x00005555560f8b14 in std::default_delete<grpc_core::RegisteredMetricCallback>::operator() (this=0x7fff8411cdd8, __ptr=0x7fff8407c230) at /usr/include/c++/9/bits/unique_ptr.h:81
#11 0x00005555560f6213 in std::unique_ptr<grpc_core::RegisteredMetricCallback, std::default_delete<grpc_core::RegisteredMetricCallback> >::reset (this=0x7fff8411cdd8, __p=0x7fff8407c230)
    at /usr/include/c++/9/bits/unique_ptr.h:402
#12 0x0000555556221c48 in grpc_core::GrpcXdsClient::Orphaned (this=0x7fff8411cbd0) at external/com_github_grpc_grpc/src/core/xds/grpc/xds_client_grpc.cc:322
#13 0x0000555556174ae9 in grpc_core::DualRefCounted<grpc_core::XdsClient, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff8411cbd0, location=..., reason=0x5555571d0f9a "DropStats")
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:127
#14 0x0000555556358473 in grpc_core::RefCountedPtr<grpc_core::XdsClient>::reset (this=0x7fff843878e8, location=..., reason=0x5555571d0f9a "DropStats", value=0x0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:132
#15 0x0000555556356bd5 in grpc_core::XdsClusterDropStats::~XdsClusterDropStats (this=0x7fff843878d0, __in_chrg=<optimized out>) at ext
8000
ernal/com_github_grpc_grpc/src/core/xds/xds_client/xds_client_stats.cc:69
#16 0x0000555556356c3e in grpc_core::XdsClusterDropStats::~XdsClusterDropStats (this=0x7fff843878d0, __in_chrg=<optimized out>) at external/com_github_grpc_grpc/src/core/xds/xds_client/xds_client_stats.cc:70
#17 0x0000555556176121 in grpc_core::UnrefDelete::operator()<grpc_core::XdsClusterDropStats const> (this=0x7fff843878d0, p=0x7fff843878d0) at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted.h:224
#18 0x0000555556174530 in grpc_core::RefCounted<grpc_core::XdsClusterDropStats, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff843878d0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted.h:366
#19 0x0000555556172e11 in grpc_core::RefCountedPtr<grpc_core::XdsClusterDropStats>::~RefCountedPtr (this=0x7fff90428240, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:113
#20 0x000055555617196c in grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker (this=0x7fff90428200, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_impl.cc:241
#21 0x00005555561719d8 in grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker (this=0x7fff90428200, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_impl.cc:241
#22 0x0000555556019884 in grpc_core::UnrefDelete::operator()<grpc_core::LoadBalancingPolicy::SubchannelPicker> (this=0x7fff90428200, p=0x7fff90428200)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted.h:224
#23 0x0000555556015b38 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::WeakUnref (this=0x7fff90428200)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:223
#24 0x0000555556011020 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff90428200)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:106
#25 0x000055555600a227 in grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker>::~RefCountedPtr (this=0x7fff843877c8, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:113
#26 0x000055555603f852 in grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker (this=0x7fff843877b0, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/outlier_detection/outlier_detection.cc:372
#27 0x000055555603f87e in grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker (this=0x7fff843877b0, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/outlier_detection/outlier_detection.cc:372
#28 0x0000555556019884 in grpc_core::UnrefDelete::operator()<grpc_core::LoadBalancingPolicy::SubchannelPicker> (this=0x7fff843877b0, p=0x7fff843877b0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted.h:224
#29 0x0000555556015b38 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::WeakUnref (this=0x7fff843877b0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:223
#30 0x0000555556011020 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff843877b0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:106
#31 0x000055555600a227 in grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker>::~RefCountedPtr (this=0x7fff9040a190, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:113
#32 0x0000555556183fa6 in std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> >::~pair (
    this=0x7fff9040a170, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/stl_pair.h:208
#33 0x0000555556183fd6 in __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::destroy<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > (this=0x7fff90407978, __p=0x7fff9040a170) at /usr/include/c++/9/ext/new_allocator.h:152
#34 0x0000555556183ced in std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_co--Type <RET> for more, q to quit, c to continue without paging--
re::LoadBalancingPolicy::SubchannelPicker> > > > >::destroy<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > (__a=..., __p=0x7fff9040a170) at /usr/include/c++/9/bits/alloc_traits.h:496
#35 0x000055555618365d in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::_M_destroy_node (this=0x7fff90407978, __p=0x7fff9040a150) at /usr/include/c++/9/bits/stl_tree.h:642
#36 0x0000555556182e99 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::_M_drop_node (this=0x7fff90407978, __p=0x7fff9040a150) at /usr/include/c++/9/bits/stl_tree.h:650
#37 0x0000555556182b0e in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::_M_erase (this=0x7fff90407978, __x=0x7fff9040a150) at /usr/include/c++/9/bits/stl_tree.h:1920
#38 0x0000555556182752 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > >, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::~_Rb_tree (this=0x7fff90407978, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/stl_tree.h:1000
#39 0x0000555556182634 in std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker>, std::less<void>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker> > > >::~map (
    this=0x7fff90407978, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/stl_map.h:300
#40 0x00005555561811ba in grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker (this=0x7fff90407960, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_manager.cc:121
#41 0x00005555561811e6 in grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker (this=0x7fff90407960, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_manager.cc:121
#42 0x0000555556019884 in grpc_core::UnrefDelete::operator()<grpc_core::LoadBalancingPolicy::SubchannelPicker> (this=0x7fff90407960, p=0x7fff90407960)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted.h:224
#43 0x0000555556015b38 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::WeakUnref (this=0x7fff90407960)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:223
#44 0x0000555556011020 in grpc_core::DualRefCounted<grpc_core::LoadBalancingPolicy::SubchannelPicker, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff90407960)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:106
#45 0x000055555600a227 in grpc_core::RefCountedPtr<grpc_core::LoadBalancingPolicy::SubchannelPicker>::~RefCountedPtr (this=0x7fffffffc710, __in_chrg=<optimized out>)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:113
#46 0x0000555556ab1151 in grpc_core::ClientChannelFilter::StartTransportOpLocked (this=0x7fff902bc3d0, op=0x55555814eae0) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1740
#47 0x0000555556ab13c6 in grpc_core::ClientChannelFilter::<lambda()>::operator()(void) const (__closure=0x7fffffffc8e0) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1765
#48 0x0000555556abc419 in std::_Function_handler<void(), grpc_core::ClientChannelFilter::StartTransportOp(grpc_channel_element*, grpc_transport_op*)::<lambda()> >::_M_invoke(const std::_Any_data &) (
    __functor=...) at /usr/include/c++/9/bits/std_function.h:300
#49 0x000055555573cb3c in std::function<void ()>::operator()() const (this=0x7fffffffc8e0) at /usr/include/c++/9/bits/std_function.h:688
#50 0x0000555556d09f7f in grpc_core::WorkSerializer::LegacyWorkSerializer::Run(std::function<void ()>, grpc_core::DebugLocation const&) (this=0x7fff902bc6c0, callback=..., location=...)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/work_serializer.cc:152
#51 0x0000555556d0b94a in grpc_core::WorkSerializer::Run(std::function<void ()>, grpc_core::DebugLocation const&) (this=0x7fff902bc6b0, callback=..., location=...)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/work_serializer.cc:527
#52 0x0000555556ab153b in grpc_core::ClientChannelFilter::StartTransportOp (elem=0x7fff902bc390, op=0x55555814eae0) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1763
#53 0x0000555556b630e8 in grpc_channel_next_op (elem=0x7fff902bc380, op=0x55555814eae0) at external/com_github_grpc_grpc/src/core/lib/channel/channel_stack.cc:279
#54 0x0000555555fe6e41 in grpc_core::promise_filter_detail::ChannelFilterMethods::StartTransportOp (elem=0x7fff902bc380, op=0x55555814eae0)
    at external/com_github_grpc_grpc/src/core/lib/channel/promise_based_filter.h:1601
#55 0x0000555556a6861d in grpc_core::LegacyChannel::Orphaned (this=0x7fff90233c50) at external/com_github_grpc_grpc/src/core/lib/surface/legacy_channel.cc:173
#56 0x0000555556009f85 in grpc_core::DualRefCounted<grpc_core::UnstartedCallDestination, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x7fff90233c50)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:103
#57 0x000055555600c622 in grpc_core::RefCountedPtr<grpc_core::Channel>::reset (this=0x7fff902bcb38, value=0x0) at external/com_github_grpc_grpc/src/core/lib/gprpp/ref_counted_ptr.h:127
#58 0x00005555560d12ef in grpc_core::(anonymous namespace)::RlsLb::RlsChannel::Orphan (this=0x7fff902bcb10) at external/com_github_grpc_grpc/src/core/load_balancing/rls/rls.cc:1686
#59 0x00005555560e229e in grpc_core::OrphanableDelete::operator()<grpc_core::(anonymous namespace)::RlsLb::RlsChannel> (this=0x7fff902bc078, p=0x7fff902bcb10)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
#60 0x00005555560dd4af in std::unique_ptr<grpc_core::(anonymous namespace)::RlsLb::RlsChannel, grpc_core::OrphanableDelete>::reset (this=0x7fff902bc078, __p=0x7fff902bcb10)
    at /usr/include/c++/9/bits/unique_ptr.h:402
#61 0x00005555560d6177 in grpc_core::(anonymous namespace)::RlsLb::ShutdownLocked (this=0x7fff902bbf50) at external/com_github_grpc_grpc/src/core/load_balancing/rls/rls.cc:2151
#62 0x0000555556cf4908 in grpc_core::LoadBalancingPolicy::Orphan (this=0x7fff902bbf50) at external/com_github_grpc_grpc/src/core/load_balancing/lb_policy.cc:49
#63 0x0000555556010cf3 in grpc_core::OrphanableDelete::operator()<grpc_core::LoadBalancingPolicy> (this=0x7fff902bbe58, p=0x7fff902bbf50) at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
--Type <RET> for more, q to quit, c to continue without paging--
#64 0x000055555600c48b in std::unique_ptr<grpc_core::LoadBalancingPolicy, grpc_core::OrphanableDelete>::reset (this=0x7fff902bbe58, __p=0x7fff902bbf50) at /usr/include/c++/9/bits/unique_ptr.h:402
#65 0x0000555556b13547 in grpc_core::ChildPolicyHandler::ShutdownLocked (this=0x7fff902bbe00) at external/com_github_grpc_grpc/src/core/load_balancing/child_policy_handler.cc:145
#66 0x0000555556cf4908 in grpc_core::LoadBalancingPolicy::Orphan (this=0x7fff902bbe00) at external/com_github_grpc_grpc/src/core/load_balancing/lb_policy.cc:49
#67 0x0000555556010cf3 in grpc_core::OrphanableDelete::operator()<grpc_core::LoadBalancingPolicy> (this=0x7fff90341b40, p=0x7fff902bbe00) at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
#68 0x000055555600c48b in std::unique_ptr<grpc_core::LoadBalancingPolicy, grpc_core::OrphanableDelete>::reset (this=0x7fff90341b40, __p=0x7fff902bbe00) at /usr/include/c++/9/bits/unique_ptr.h:402
#69 0x000055555617b045 in grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild::Orphan (this=0x7fff90341b00)
    at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_manager.cc:423
#70 0x000055555617e046 in grpc_core::OrphanableDelete::operator()<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild> (this=0x7fff90342530, p=0x7fff90341b00)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
#71 0x000055555617ce10 in std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete>::~unique_ptr (this=0x7fff90342530, __in_chrg=<optimized out>)
    at /usr/include/c++/9/bits/unique_ptr.h:292
#72 0x0000555556180b84 in std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >::~pair (this=0x7fff90342510, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/stl_pair.h:208
#73 0x0000555556180bb0 in __gnu_cxx::new_allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::destroy<std::pair<std::__cxx11::basic_string<char> const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > (this=0x7fff9022c7a0, __p=0x7fff90342510) at /usr/include/c++/9/ext/new_allocator.h:152
#74 0x00005555561806d7 in std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > > >::destroy<std::pair<std::__cxx11::basic_string<char> const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > (__a=..., __p=0x7fff90342510) at /usr/include/c++/9/bits/alloc_traits.h:496
#75 0x000055555617fb71 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::_M_destroy_node (this=0x7fff9022c7a0, __p=0x7fff903424f0) at /usr/include/c++/9/bits/stl_tree.h:642
#76 0x000055555617e965 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::_M_drop_node (this=0x7fff9022c7a0, __p=0x7fff903424f0) at /usr/include/c++/9/bits/stl_tree.h:650
#77 0x000055555617da44 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::_M_erase (this=0x7fff9022c7a0, __x=0x7fff903424f0) at /usr/include/c++/9/bits/stl_tree.h:1920
#78 0x000055555617da21 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::_M_erase (this=0x7fff9022c7a0, __x=0x7fff90253d80) at /usr/include/c++/9/bits/stl_tree.h:1918
#79 0x000055555617da9c in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::clear (this=0x7fff9022c7a0) at /usr/include/c++/9/bits/stl_tree.h:1271
#80 0x000055555617c756 in std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unique_ptr<grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterChild, grpc_core::OrphanableDelete> > > >::clear (this=0x7fff9022c7a0) at /usr/include/c++/9/bits/stl_map.h:1133
#81 0x00005555561799ae in grpc_core::(anonymous namespace)::XdsClusterManagerLb::ShutdownLocked (this=0x7fff9022c750) at external/com_github_grpc_grpc/src/core/load_balancing/xds/xds_cluster_manager.cc:261
#82 0x0000555556cf4908 in grpc_core::LoadBalancingPolicy::Orphan (this=0x7fff9022c750) at external/com_github_grpc_grpc/src/core/load_balancing/lb_policy.cc:49
#83 0x0000555556010cf3 in grpc_core::OrphanableDelete::operator()<grpc_core::LoadBalancingPolicy> (this=0x7fffa802f798, p=0x7fff9022c750) at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
#84 0x000055555600c48b in std::unique_ptr<grpc_core::LoadBalancingPolicy, grpc_core::OrphanableDelete>::reset (this=0x7fffa802f798, __p=0x7fff9022c750) at /usr/include/c++/9/bits/unique_ptr.h:402
#85 0x0000555556b13547 in grpc_core::ChildPolicyHandler::ShutdownLocked (this=0x7fffa802f740) at external/com_github_grpc_grpc/src/core/load_balancing/child_policy_handler.cc:145
#86 0x0000555556cf4908 in grpc_core::LoadBalancingPolicy::Orphan (this=0x7fffa802f740) at external/com_github_grpc_grpc/src/core/load_balancing/lb_policy.cc:49
#87 0x0000555556010cf3 in grpc_core::OrphanableDelete::operator()<grpc_core::LoadBalancingPolicy> (this=0x55555814daa0, p=0x7fffa802f740) at external/com_github_grpc_grpc/src/core/lib/gprpp/orphanable.h:60
#88 0x000055555600c48b in std::unique_ptr<grpc_core::LoadBalancingPolicy, grpc_core::OrphanableDelete>::reset (this=0x55555814daa0, __p=0x7fffa802f740) at /usr/include/c++/9/bits/unique_ptr.h:402
#89 0x0000555556ab019a in grpc_core::ClientChannelFilter::DestroyResolverAndLbPolicyLocked (this=0x55555814d910) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1576
#90 0x0000555556ab0f0c in grpc_core::ClientChannelFilter::StartTransportOpLocked (this=0x55555814d910, op=0x555558144e60) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1722
#91 0x0000555556ab13c6 in grpc_core::ClientChannelFilter::<lambda()>::operator()(void) const (__closure=0x7fffffffd4c0) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1765
--Type <RET> for more, q to quit, c to continue without paging--
#92 0x0000555556abc419 in std::_Function_handler<void(), grpc_core::ClientChannelFilter::StartTransportOp(grpc_channel_element*, grpc_transport_op*)::<lambda()> >::_M_invoke(const std::_Any_data &) (
    __functor=...) at /usr/include/c++/9/bits/std_function.h:300
#93 0x000055555573cb3c in std::function<void ()>::operator()() const (this=0x7fffffffd4c0) at /usr/include/c++/9/bits/std_function.h:688
#94 0x0000555556d09f7f in grpc_core::WorkSerializer::LegacyWorkSerializer::Run(std::function<void ()>, grpc_core::DebugLocation const&) (this=0x555558142790, callback=..., location=...)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/work_serializer.cc:152
#95 0x0000555556d0b94a in grpc_core::WorkSerializer::Run(std::function<void ()>, grpc_core::DebugLocation const&) (this=0x5555581444a0, callback=..., location=...)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/work_serializer.cc:527
#96 0x0000555556ab153b in grpc_core::ClientChannelFilter::StartTransportOp (elem=0x55555814d8d0, op=0x555558144e60) at external/com_github_grpc_grpc/src/core/client_channel/client_channel_filter.cc:1763
#97 0x0000555556b630e8 in grpc_channel_next_op (elem=0x55555814d8c0, op=0x555558144e60) at external/com_github_grpc_grpc/src/core/lib/channel/channel_stack.cc:279
#98 0x0000555555fe6e41 in grpc_core::promise_filter_detail::ChannelFilterMethods::StartTransportOp (elem=0x55555814d8c0, op=0x555558144e60)
    at external/com_github_grpc_grpc/src/core/lib/channel/promise_based_filter.h:1601
#99 0x0000555556a6861d in grpc_core::LegacyChannel::Orphaned (this=0x55555814dfd0) at external/com_github_grpc_grpc/src/core/lib/surface/legacy_channel.cc:173
#100 0x0000555556009f85 in grpc_core::DualRefCounted<grpc_core::UnstartedCallDestination, grpc_core::PolymorphicRefCount, grpc_core::UnrefDelete>::Unref (this=0x55555814dfd0)
    at external/com_github_grpc_grpc/src/core/lib/gprpp/dual_ref_counted.h:103
#101 0x0000555556456c96 in grpc_channel_destroy_internal (channel=0x55555814dfd0) at external/com_github_grpc_grpc/src/core/lib/surface/channel.h:156
#102 0x0000555556bf8119 in grpc_channel_destroy (channel=0x55555814dfd0) at external/com_github_grpc_grpc/src/core/lib/surface/channel.cc:99
#103 0x0000555555f6e373 in grpc::Channel::~Channel (this=0x555558143ce0, __in_chrg=<optimized out>) at external/com_github_grpc_grpc/src/cpp/client/channel_cc.cc:59
#104 0x0000555555f6e464 in grpc::Channel::~Channel (this=0x555558143ce0, __in_chrg=<optimized out>) at external/com_github_grpc_grpc/src/cpp/client/channel_cc.cc:69
#105 0x0000555555f75ea3 in std::_Sp_counted_ptr<grpc::Channel*, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x555558145190) at /usr/include/c++/9/bits/shared_ptr_base.h:377
#106 0x00005555556d33a6 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x555558145190) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#107 0x00005555556d2fab in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7fffffffd918, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#108 0x00005555557c4f74 in std::__shared_ptr<grpc::Channel, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7fffffffd910, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#109 0x0000555555e82cb1 in std::__shared_ptr<grpc::Channel, (__gnu_cxx::_Lock_policy)2>::reset (this=0x7fffffffd950) at /usr/include/c++/9/bits/shared_ptr_base.h:1287
#110 0x0000555555e8b294 in ChannelPoller::~ChannelPoller (this=0x555558142cb0, __in_chrg=<optimized out>) at e2e-examples/gcs/benchmark/channel_poller.cc:35
#111 0x0000555555e836fc in std::default_delete<ChannelPoller>::operator() (this=0x55555803aa18, __ptr=0x555558142cb0) at /usr/include/c++/9/bits/unique_ptr.h:81
#112 0x0000555555e82bf4 in std::unique_ptr<ChannelPoller, std::default_delete<ChannelPoller> >::~unique_ptr (this=0x55555803aa18, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/unique_ptr.h:292
#113 0x0000555555e80b7e in ConstChannelPool::~ConstChannelPool (this=0x55555803a9e0, __in_chrg=<optimized out>) at e2e-examples/gcs/benchmark/channel_policy.cc:31
#114 0x0000555555e8b0ad in __gnu_cxx::new_allocator<ConstChannelPool>::destroy<ConstChannelPool> (this=0x55555803a9e0, __p=0x55555803a9e0) at /usr/include/c++/9/ext/new_allocator.h:152
#115 0x0000555555e8af81 in std::allocator_traits<std::allocator<ConstChannelPool> >::destroy<ConstChannelPool> (__a=..., __p=0x55555803a9e0) at /usr/include/c++/9/bits/alloc_traits.h:496
#116 0x0000555555e8ad3d in std::_Sp_counted_ptr_inplace<ConstChannelPool, std::allocator<ConstChannelPool>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x55555803a9d0)
    at /usr/include/c++/9/bits/shared_ptr_base.h:557
#117 0x00005555556d33a6 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x55555803a9d0) at /usr/include/c++/9/bits/shared_ptr_base.h:155
#118 0x00005555556d2fab in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7fffffffdb18, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:730
#119 0x0000555555e7596c in std::__shared_ptr<StorageStubProvider, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7fffffffdb10, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr_base.h:1169
#120 0x0000555555e7
8000
598c in std::shared_ptr<StorageStubProvider>::~shared_ptr (this=0x7fffffffdb10, __in_chrg=<optimized out>) at /usr/include/c++/9/bits/shared_ptr.h:103
#121 0x0000555555e70be9 in GrpcRunner::Run (this=0x55555802d560) at e2e-examples/gcs/benchmark/grpc_runner.cc:130
#122 0x00005555556d1f41 in main (argc=13, argv=0x7fffffffe288) at e2e-examples/gcs/benchmark/main.cc:72

markdroth
markdroth reviewed Aug 13, 2024
View reviewed changes
Copy link
Member
@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

This looks like the right approach!

Please let me know if you have any questions. Thanks!

markdroth
markdroth reviewed Aug 14, 2024
View reviewed changes
Copy link
Member
@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

This looks much cleaner! Just a few comments remaining.

Please let me know if you have any questions. Thanks!

markdroth
markdroth approved these changes Aug 15, 2024
View reviewed changes
Copy link
Member
@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

Looks great!

yashykt added a commit to yashykt/grpc that referenced this pull request Aug 15, 2024
@yashykt
[RlsLB] Fix Deadlock (grpc#37459)
86c6d07 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
@yashykt yashykt mentioned this pull request Aug 15, 2024
Merged
yashykt added a commit to yashykt/grpc that referenced this pull request Aug 15, 2024
@yashykt
[RlsLB] Fix Deadlock (grpc#37459)
2f15b6e 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
@yashykt yashykt mentioned this pull request Aug 15, 2024
Merged
yashykt added a commit to yashykt/grpc that referenced this pull request Aug 15, 2024
@yashykt
[RlsLB] Fix Deadlock (grpc#37459)
b911d24 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
@yashykt yashykt mentioned this pull request Aug 15, 2024
Merged
yashykt added a commit to yashykt/grpc that referenced this pull request Aug 15, 2024
@yashykt
[RlsLB] Fix Deadlock (grpc#37459)
1b88368 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
@yashykt yashykt mentioned this pull request Aug 15, 2024
Merged
drfloob pushed a commit that referenced this pull request Aug 15, 2024
@yashykt
[RlsLB] Fix Deadlock (#37459) (#37502)
e53a314 
Backport #37459 to 1.66
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the
`RlsLB` policy while removing a callback from the gRPC OpenTelemetry
plugin, which is a lock ordering inversion. The correct order is
`OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like
RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be
unregistered when the corresponding component object is
orphaned/destroyed (unreffing). Also, note that removing callbacks
requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we
remove the callback inside `RlsLb` from outside the critical region, but
`RlsLb` owns refs to child policies which in turn hold refs to
`XdsClient`. The lock ordering inversion occurred due to unreffing child
policies within the critical region.

This PR is an alternative fix to this problem. Original fix in
#37425. Verified that it fixes the bug.
yashykt added a commit that referenced this pull request Aug 16, 2024
@yashykt
[RlsLB] Fix Deadlock (#37459) (#37503)
bd677a0 
Backport #37459 to v1.65
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDr
F438
opStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the
`RlsLB` policy while removing a callback from the gRPC OpenTelemetry
plugin, which is a lock ordering inversion. The correct order is
`OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like
RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be
unregistered when the corresponding component object is
orphaned/destroyed (unreffing). Also, note that removing callbacks
requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we
remove the callback inside `RlsLb` from outside the critical region, but
`RlsLb` owns refs to child policies which in turn hold refs to
`XdsClient`. The lock ordering inversion occurred due to unreffing child
policies within the critical region.

This PR is an alternative fix to this problem. Original fix in
#37425. Verified that it fixes the bug.

Closes #37459

COPYBARA_INTEGRATE_REVIEW=#37459 from
yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427




<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
yashykt added a commit that referenced this pull request Aug 16, 2024
@yashykt
[RlsLB] Fix Deadlock (#37459) (#37504)
4f10fd0 
Backport #37459 to 1.64.x
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the
`RlsLB` policy while removing a callback from the gRPC OpenTelemetry
plugin, which is a lock ordering inversion. The correct order is
`OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like
RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be
unregistered when the corresponding component object is
orphaned/destroyed (unreffing). Also, note that removing callbacks
requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we
remove the callback inside `RlsLb` from outside the critical region, but
`RlsLb` owns refs to child policies which in turn hold refs to
`XdsClient`. The lock ordering inversion occurred due to unreffing child
policies within the critical region.

This PR is an alternative fix to this problem. Original fix in
#37425. Verified that it fixes the bug.
yashykt added a commit that referenced this pull request Aug 16, 2024
@yashykt
[RlsLB] Fix Deadlock (#37459) (#37505)
9e4a38a 
Backport #37459 to 1.63
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the
`RlsLB` policy while removing a callback from the gRPC OpenTelemetry
plugin, which is a lock ordering inversion. The correct order is
`OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like
RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be
unregistered when the corresponding component object is
orphaned/destroyed (unreffing). Also, note that removing callbacks
requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we
remove the callback inside `RlsLb` from outside the critical region, but
`RlsLb` owns refs to child policies which in turn hold refs to
`XdsClient`. The lock ordering inversion occurred due to unreffing child
policies within the critical region.

This PR is an alternative fix to this problem. Original fix in
#37425. Verified that it fixes the bug.
sourabhsinghs pushed a commit to sourabhsinghs/grpc that referenced this pull request Sep 26, 2024
@yashykt @sourabhsinghs
[RlsLB] Fix Deadlock (grpc#37459)
9daf0a9 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
paulosjca pushed a commit to paulosjca/grpc that referenced this pull request Nov 25, 2024
@yashykt @paulosjca
[RlsLB] Fix Deadlock (grpc#37459)
2b59b7e 
Internal bug: b/357864682

A lock ordering inversion was noticed with the following stacks -
```
[mutex.cc : 1418] RAW: Potential Mutex deadlock:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be968c5 grpc::internal::OpenTelemetryPluginImpl::RemoveCallback()
        @ 0x564f4cd097b8 grpc_core::RegisteredMetricCallback::~RegisteredMetricCallback()
        @ 0x564f4c1f1216 std::default_delete<>::operator()()
        @ 0x564f4c1f157f std::__uniq_ptr_impl<>::reset()
        @ 0x564f4c1ee967 std::unique_ptr<>::reset()
        @ 0x564f4c352f44 grpc_core::GrpcXdsClient::Orphaned()
        @ 0x564f4c25dad1 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c4653ed grpc_core::RefCountedPtr<>::reset()
        @ 0x564f4c463c73 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c463d02 grpc_core::XdsClusterDropStats::~XdsClusterDropStats()
        @ 0x564f4c25efa9 grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c25d5f0 grpc_core::RefCounted<>::Unref()
        @ 0x564f4c25c2d9 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c25b1d8 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c25b240 grpc_core::(anonymous namespace)::XdsClusterImplLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c14e958 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c14e980 grpc_core::(anonymous namespace)::OutlierDetectionLb::Picker::~Picker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()
        @ 0x564f4c124fb8 grpc_core::DualRefCounted<>::Unref()
        @ 0x564f4c11f029 grpc_core::RefCountedPtr<>::~RefCountedPtr()
        @ 0x564f4c26bafc std::pair<>::~pair()
        @ 0x564f4c26bb28 __gnu_cxx::new_allocator<>::destroy<>()
        @ 0x564f4c26b88f std::allocator_traits<>::destroy<>()
        @ 0x564f4c26b297 std::_Rb_tree<>::_M_destroy_node()
        @ 0x564f4c26abfb std::_Rb_tree<>::_M_drop_node()
        @ 0x564f4c26a926 std::_Rb_tree<>::_M_erase()
        @ 0x564f4c26a6f0 std::_Rb_tree<>::~_Rb_tree()
        @ 0x564f4c26a62a std::map<>::~map()
        @ 0x564f4c2691a4 grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c2691cc grpc_core::(anonymous namespace)::XdsClusterManagerLb::ClusterPicker::~ClusterPicker()
        @ 0x564f4c12c71a grpc_core::UnrefDelete::operator()<>()
        @ 0x564f4c1292ac grpc_core::DualRefCounted<>::WeakUnref()

[mutex.cc : 1428] RAW: Acquiring absl::Mutex 0x564f4f22ad40 while holding  0x7f939834bb70; a cycle in the historical lock ordering graph has been observed
[mutex.cc : 1432] RAW: Cycle:
[mutex.cc : 1446] RAW: mutex@0x564f4f22ad40 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4be96124 grpc::internal::OpenTelemetryPluginImpl::AddCallback()
        @ 0x564f4cd096f0 grpc_core::RegisteredMetricCallback::RegisteredMetricCallback()
        @ 0x564f4c1f111b std::make_unique<>()
        @ 0x564f4c3564b0 grpc_core::GlobalStatsPluginRegistry::StatsPluginGroup::RegisterCallback<>()
        @ 0x564f4c352dea grpc_core::GrpcXdsClient::GrpcXdsClient()
        @ 0x564f4c355bc6 grpc_core::MakeRefCounted<>()
        @ 0x564f4c3525f2 grpc_core::GrpcXdsClient::GetOrCreate()
        @ 0x564f4c28f8f8 grpc_core::(anonymous namespace)::XdsResolver::StartLocked()
        @ 0x564f4c2f5f82 grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartXdsResolver()
        @ 0x564f4c2f515d grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::ZoneQueryDone()
        @ 0x564f4c2f496b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()::{lambda()#1}::operator()()
        @ 0x564f4c2f80f6 std::__invoke_impl<>()
        @ 0x564f4c2f7b9d _ZSt10__invoke_rIvRZZN9grpc_core12_GLOBAL__N_124GoogleCloud2ProdResolver11StartLockedEvENUlNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEN4absl12lts_202401168StatusOrIS8_EEE_clES8_SC_EUlvE_J...
        @ 0x564f4c2f748c std::_Function_handler<>::_M_invoke()
        @ 0x564f4b8ad682 std::function<>::operator()()
        @ 0x564f4cd1c6bf grpc_core::WorkSerializer::LegacyWorkSerializer::Run()
        @ 0x564f4cd1dae4 grpc_core::WorkSerializer::Run()
        @ 0x564f4c2f4b0b grpc_core::(anonymous namespace)::GoogleCloud2ProdResolver::StartLocked()::{lambda()#1}::operator()()
        @ 0x564f4c2f8dc7 absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c2f8cb8 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c2f8b16 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c2f8a0c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4c2fb88d absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4c2fb1f3 grpc_core::GcpMetadataQuery::OnDone()
        @ 0x564f4cd75a72 exec_ctx_run()
        @ 0x564f4cd75ba9 grpc_core::ExecCtx::Flush()
        @ 0x564f4cc8ee1d end_worker()
        @ 0x564f4cc8f304 pollset_work()
        @ 0x564f4cc5dcaf pollset_work()
        @ 0x564f4cc69220 grpc_pollset_work()
        @ 0x564f4cbe7733 cq_pluck()
        @ 0x564f4cbe7ad5 grpc_completion_queue_pluck
        @ 0x564f4bc61d96 grpc::CompletionQueue::Pluck()
        @ 0x564f4bfdb055 grpc::ClientReader<>::ClientReader<>()
        @ 0x564f4bfd6035 grpc::internal::ClientReaderFactory<>::Create<>()
        @ 0x564f4bfc322b google::storage::v2::Storage::Stub::ReadObjectRaw()
        @ 0x564f4bf9934b google::storage::v2::Storage::Stub::ReadObject()

[mutex.cc : 1446] RAW: mutex@0x7f939834bb70 stack:
        @ 0x564f4ce62fe5 absl::lts_20240116::DebugOnlyDeadlockCheck()
        @ 0x564f4ce632dc absl::lts_20240116::Mutex::Lock()
        @ 0x564f4be5886c absl::lts_20240116::MutexLock::MutexLock()
        @ 0x564f4c1ce9eb grpc_core::(anonymous namespace)::RlsLb::RlsLb()::{lambda()#1}::operator()()
        @ 0x564f4c1e794c absl::lts_20240116::base_internal::Callable::Invoke<>()
        @ 0x564f4c1e72c1 absl::lts_20240116::base_internal::invoke<>()
        @ 0x564f4c1e6af1 absl::lts_20240116::internal_any_invocable::InvokeR<>()
        @ 0x564f4c1e5d6c absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
        @ 0x564f4be9d0c8 absl::lts_20240116::internal_any_invocable::Impl<>::operator()()
        @ 0x564f4be9b4ff grpc_core::RegisteredMetricCallback::Run()
        @ 0x564f4bea07ae grpc::internal::OpenTelemetryPluginImpl::CallbackGaugeState<>::CallbackGaugeCallback()
        @ 0x564f4bf844de opentelemetry::v1::sdk::metrics::ObservableRegistry::Observe()
        @ 0x564f4bf56529 opentelemetry::v1::sdk::metrics::Meter::Collect()
        @ 0x564f4bf8c1d5 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5ac opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::operator()()
        @ 0x564f4bf8c5e8 opentelemetry::v1::nostd::function_ref<>::BindTo<>()::{lambda()#1}::_FUN()
        @ 0x564f4bf7604d opentelemetry::v1::nostd::function_ref<>::operator()()
        @ 0x564f4bf74ad9 opentelemetry::v1::sdk::metrics::MeterContext::ForEachMeter()
        @ 0x564f4bf8c457 opentelemetry::v1::sdk::metrics::MetricCollector::Collect()
        @ 0x564f4bf4a7fe opentelemetry::v1::sdk::metrics::MetricReader::Collect()
        @ 0x564f4bed5e24 opentelemetry::v1::exporter::metrics::PrometheusCollector::Collect()
        @ 0x564f4bef004f prometheus::detail::CollectMetrics()
        @ 0x564f4beec26d prometheus::detail::MetricsHandler::handleGet()
        @ 0x564f4bf1cd8b CivetServer::requestHandler()
        @ 0x564f4bf35e7b handle_request
        @ 0x564f4bf29534 handle_request_stat_log
        @ 0x564f4bf39b3f process_new_connection
        @ 0x564f4bf3a448 worker_thread_run
        @ 0x564f4bf3a57f worker_thread
        @ 0x7f93e9137ea7 start_thread

[mutex.cc : 1454] RAW: dying due to potential deadlock
Aborted
```

From the stack, it looks like we are ending up holding a lock to the `RlsLB` policy while removing a callback from the gRPC OpenTelemetry plugin, which is a lock ordering inversion. The correct order is `OpenTelemetry` -> `gRPC OpenTelemetry plugin` -> `gRPC Component like RLS/xDSClient`.

A common pattern we employ for metrics is for the callbacks to be unregistered when the corresponding component object is orphaned/destroyed (unreffing). Also, note that removing callbacks requires a lock in `gRPC OpenTelemetry plugin`. To avoid deadlocks, we remove the callback inside `RlsLb` from outside the critical region, but `RlsLb` owns refs to child policies which in turn hold refs to `XdsClient`. The lock ordering inversion occurred due to unreffing child policies within the critical region.

This PR is an alternative fix to this problem. Original fix in grpc#37425.
Verified that it fixes the bug.

Closes grpc#37459

COPYBARA_INTEGRATE_REVIEW=grpc#37459 from yashykt:FixDeadlocks ec7fbcf
PiperOrigin-RevId: 663360427
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markdroth markdroth markdroth approved these changes

Assignees
No one assigned
Labels
bloat/none lang/core per-call-memory/neutral per-channel-memory/neutral release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yashykt @markdroth
0