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.

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

add examples #1

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

add examples #1

wants to merge 1 commit into from

Conversation

chjasonwu
Copy link
Owner

No description provided.

chjasonwu pushed a commit that referenced this pull request Oct 31, 2024
…grpc#37225)

Noticed on a Core End2End test failure https://btx.cloud.google.com/invocations/dc3bf84d-e6ed-4b32-a24c-12489f981e46/targets/%2F%2Ftest%2Fcore%2Fend2end:cancel_with_status_test@poller%3Depoll1;config=56f5b09615e325097b100b58c41171656571290519a83c5d89a6067ef0283d46/log

```
F0000 00:00:1721017820.001684      87 tcp_server_posix.cc:354] Check failed: !s->shutdown
*** Check failure stack trace: ***
    @     0x7f32578da0e4  absl::lts_20240116::log_internal::LogMessage::SendToLog()
    @     0x7f32578d9a94  absl::lts_20240116::log_internal::LogMessage::Flush()
    @     0x7f32578da589  absl::lts_20240116::log_internal::LogMessageFatal::~LogMessageFatal()
    @     0x7f3257e340a1  tcp_server_unref()
    @     0x7f3258fcba8e  grpc_core::Chttp2ServerListener::ActiveConnection::~ActiveConnection()
    @     0x7f3258fd19e7  grpc_event_engine::experimental::MemoryAllocator::New<>()::Wrapper::~Wrapper()
    @     0x7f3258fcc998  grpc_core::Chttp2ServerListener::OnAccept()
    @     0x7f3257e34962  absl::lts_20240116::internal_any_invocable::LocalInvoker<>()
    @     0x7f3257da6475  grpc_event_engine::experimental::PosixEngineListenerImpl::AsyncConnectionAcceptor::NotifyOnAccept()::$_1::operator()()
    @     0x7f3257da4437  grpc_event_engine::experimental::PosixEngineListenerImpl::AsyncConnectionAcceptor::NotifyOnAccept()
    @     0x7f3257da5fef  absl::lts_20240116::base_internal::Callable::Invoke<>()
    @     0x7f3257dca50a  grpc_event_engine::experimental::PosixEngineClosure::Run()
    @     0x7f3257c9013e  grpc_event_engine::experimental::WorkStealingThreadPool::ThreadState::Step()
    @     0x7f3257c8fe48  grpc_event_engine::experimental::WorkStealingThreadPool::ThreadState::ThreadBody()
    @     0x7f3257c906df  grpc_event_engine::experimental::WorkStealingThreadPool::WorkStealingThreadPoolImpl::StartThread()::$_0::__invoke()
    @     0x7f32579a106c  grpc_core::(anonymous namespace)::ThreadInternalsPosix::ThreadInternalsPosix()::{lambda()#1}::__invoke()
    @     0x7f3257358609  start_thread
```

grpc#36563 changed the refcounting mechanism incorrectly and we ended up taking a ref on the tcp server outside the critical region, resulting in a time-of-check-to-time-of-use bug, where we could end up reffing the tcp server when it is already 0, i.e., when the listener has already been shutdown. This results in an attempt to destroy the tcp server twice and an eventual crash.

Closes grpc#37225

COPYBARA_INTEGRATE_REVIEW=grpc#37225 from yashykt:FixChttp2Bug bc1e8df
PiperOrigin-RevId: 654850991
chjasonwu pushed a commit that referenced this pull request Oct 31, 2024
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
chjasonwu pushed a commit that referenced this pull request Oct 31, 2024
…haned (grpc#37683)

Sample race - https://btx.cloud.google.com/invocations/0c4e65f2-3a38-4b4f-b67e-c53a4a4650ea/targets/%2F%2Ftest%2Fcore%2Fend2end:connectivity_test@poller%3Dpoll;config=2aed862ff4fd4384687d63aa95df415c7cb955355c2ab6dc6c6d7a9d123a76ec/log

```
WARNING: ThreadSanitizer: data race (pid=18)
  Write of size 8 at 0x72300000c318 by thread T29:
    #0 grpc_core::Chttp2ServerListener* std::__exchange(grpc_core::Chttp2ServerListener*&, grpc_core::Chttp2ServerListener*&) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/move.h:152:13 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x68c85)
    #1 grpc_core::Chttp2ServerListener* std::exchange(grpc_core::Chttp2ServerListener*&, grpc_core::Chttp2ServerListener*&) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/utility:287:14 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x68c05)
    grpc#2 grpc_core::RefCountedPtr::reset(grpc_core::Chttp2ServerListener*) /proc/self/cwd/./src/core/lib/gprpp/ref_counted_ptr.h:126:20 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x68b32)
    grpc#3 grpc_core::RefCountedPtr::operator=(grpc_core::RefCountedPtr&&) /proc/self/cwd/./src/core/lib/gprpp/ref_counted_ptr.h:66:5 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x54380)
    grpc#4 grpc_core::Chttp2ServerListener::ActiveConnection::Start(grpc_core::RefCountedPtr, std::unique_ptr, grpc_core::ChannelArgs const&) /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:615:13 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x48914)
    grpc#5 grpc_core::Chttp2ServerListener::OnAccept(void*, grpc_endpoint*, grpc_pollset*, grpc_tcp_server_acceptor*) /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:881:21 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x49ce2)
    grpc#6 CreateEventEngineListener(grpc_tcp_server*, grpc_closure*, grpc_event_engine::experimental::EndpointConfig const&, grpc_tcp_server**)::$_2::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const /proc/self/cwd/src/core/lib/iomgr/tcp_server_posix.cc:228:11 (liblibiomgr.so+0xef627)
    grpc#7 decltype(std::declval()(std::declval>>(), std::declval())) absl::lts_20240116::base_internal::Callable::Invoke>, grpc_event_engine::experimental::MemoryAllocator>(CreateEventEngineListener(grpc_tcp_server*, grpc_closure*, grpc_event_engine::experimental::EndpointConfig const&, grpc_tcp_server**)::$_2&, std::unique_ptr>&&, grpc_event_engine::experimental::MemoryAllocator&&) /proc/self/cwd/external/com_google_absl/absl/base/internal/invoke.h:185:12 (liblibiomgr.so+0xef3c2)
    grpc#8 decltype(Invoker>, grpc_event_engine::experimental::MemoryAllocator>::type::Invoke(std::declval(), std::declval>>(), std::declval())) absl::lts_20240116::base_internal::invoke>, grpc_event_engine::experimental::MemoryAllocator>(CreateEventEngineListener(grpc_tcp_server*, grpc_closure*, grpc_event_engine::experimental::EndpointConfig const&, grpc_tcp_server**)::$_2&, std::unique_ptr>&&, grpc_event_engine::experimental::MemoryAllocator&&) /proc/self/cwd/external/com_google_absl/absl/base/internal/invoke.h:212:10 (liblibiomgr.so+0xef325)
    grpc#9 void absl::lts_20240116::internal_any_invocable::InvokeR>, grpc_event_engine::experimental::MemoryAllocator, void>(CreateEventEngineListener(grpc_tcp_server*, grpc_closure*, grpc_event_engine::experimental::EndpointConfig const&, grpc_tcp_server**)::$_2&, std::unique_ptr>&&, grpc_event_engine::experimental::MemoryAllocator&&) /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:132:3 (liblibiomgr.so+0xef2b5)
    grpc#10 void absl::lts_20240116::internal_any_invocable::LocalInvoker>, grpc_event_engine::experimental::MemoryAllocator>(absl::lts_20240116::internal_any_invocable::TypeErasedState*, absl::lts_20240116::internal_any_invocable::ForwardedParameter>>::type, absl::lts_20240116::internal_any_invocable::ForwardedParameter::type) /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:310:10 (liblibiomgr.so+0xef1e2)
    grpc#11 absl::lts_20240116::internal_any_invocable::Impl>, grpc_event_engine::experimental::MemoryAllocator)>::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:868:1 (libsrc_Score_Slibposix_Uevent_Uengine.so+0xa754f)
    grpc#12 grpc_event_engine::experimental::ThreadyEventEngine::CreateListener(absl::lts_20240116::AnyInvocable>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()::operator()() /proc/self/cwd/src/core/lib/event_engine/thready_event_engine/thready_event_engine.cc:61:15 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x27cdb)
    grpc#13 decltype(std::declval>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>()()) absl::lts_20240116::base_internal::Callable::Invoke>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>(grpc_event_engine::experimental::ThreadyEventEngine::CreateListener(absl::lts_20240116::AnyInvocable>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&) /proc/self/cwd/external/com_google_absl/absl/base/internal/invoke.h:185:12 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x27c45)
    grpc#14 decltype(Invoker>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>::type::Invoke(std::declval>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>())) absl::lts_20240116::base_internal::invoke>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>(grpc_event_engine::experimental::ThreadyEventEngine::CreateListener(absl::lts_20240116::AnyInvocable>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&) /proc/self/cwd/external/com_google_absl/absl/base/internal/invoke.h:212:10 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x27bf5)
    grpc#15 void absl::lts_20240116::internal_any_invocable::InvokeR>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&, void>(grpc_event_engine::experimental::ThreadyEventEngine::CreateListener(absl::lts_20240116::AnyInvocable>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&) /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:132:3 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x27ba5)
    grpc#16 void absl::lts_20240116::internal_any_invocable::RemoteInvoker>, grpc_event_engine::experimental::MemoryAllocator)>, absl::lts_20240116::AnyInvocable, grpc_event_engine::experimental::EndpointConfig const&, std::unique_ptr>)::$_0::operator()(std::unique_ptr>, grpc_event_engine::experimental::MemoryAllocator) const::'lambda'()&>(absl::lts_20240116::internal_any_invocable::TypeErasedState*) /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:368:10 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x279cd)
    grpc#17 absl::lts_20240116::internal_any_invocable::Impl::operator()() /proc/self/cwd/external/com_google_absl/absl/functional/internal/any_invocable.h:868:1 (libtest_Score_Send2end_Slibconnectivity_Ulibrary.so+0x337ff)
    grpc#18 grpc_core::Thread::Thread(char const*, absl::lts_20240116::AnyInvocable, bool*, grpc_core::Thread::Options const&)::'lambda'(void*)::operator()(void*) const /proc/self/cwd/./src/core/lib/gprpp/thd.h:108:15 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x2e264)
    grpc#19 grpc_core::Thread::Thread(char const*, absl::lts_20240116::AnyInvocable, bool*, grpc_core::Thread::Options const&)::'lambda'(void*)::__invoke(void*) /proc/self/cwd/./src/core/lib/gprpp/thd.h:105:13 (libsrc_Score_Slibthready_Uevent_Uengine.so+0x2e1e9)
    grpc#20 grpc_core::(anonymous namespace)::ThreadInternalsPosix::ThreadInternalsPosix(char const*, void (*)(void*), void*, bool*, grpc_core::Thread::Options const&)::'lambda'(void*)::operator()(void*) const /proc/self/cwd/src/core/lib/gprpp/posix/thd.cc:148:11 (liblibgpr.so+0x1d830)
    grpc#21 grpc_core::(anonymous namespace)::ThreadInternalsPosix::ThreadInternalsPosix(char const*, void (*)(void*), void*, bool*, grpc_core::Thread::Options const&)::'lambda'(void*)::__invoke(void*) /proc/self/cwd/src/core/lib/gprpp/posix/thd.cc:118:9 (liblibgpr.so+0x1d659)

  Previous read of size 8 at 0x72300000c318 by main thread:
    #0 grpc_core::RefCountedPtr::operator!=(std::nullptr_t) const /proc/self/cwd/./src/core/lib/gprpp/ref_counted_ptr.h:192:50 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x52345)
    #1 grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState::~HandshakingState() /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:394:30 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x463ed)
    grpc#2 std::enable_if::value, grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState*>::type grpc_event_engine::experimental::MemoryAllocator::New, grpc_pollset*&, std::unique_ptr, grpc_core::ChannelArgs const&>(grpc_core::RefCountedPtr&&, grpc_pollset*&, std::unique_ptr&&, grpc_core::ChannelArgs const&)::Wrapper::~Wrapper() /proc/self/cwd/include/grpc/event_engine/memory_allocator.h:117:65 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x67a1c)
    grpc#3 std::enable_if::value, grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState*>::type grpc_event_engine::experimental::MemoryAllocator::New, grpc_pollset*&, std::unique_ptr, grpc_core::ChannelArgs const&>(grpc_core::RefCountedPtr&&, grpc_pollset*&, std::unique_ptr&&, grpc_core::ChannelArgs const&)::Wrapper::~Wrapper() /proc/self/cwd/include/grpc/event_engine/memory_allocator.h:117:27 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x67a59)
    grpc#4 void grpc_core::UnrefDelete::operator()(grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState*) const /proc/self/cwd/./src/core/lib/gprpp/ref_counted.h:224:5 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x6449d)
    grpc#5 grpc_core::InternallyRefCounted::Unref() /proc/self/cwd/./src/core/lib/gprpp/orphanable.h:132:7 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x52581)
    grpc#6 grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState::Orphan() /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:407:3 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x466ac)
    grpc#7 void grpc_core::OrphanableDelete::operator()(grpc_core::Chttp2ServerListener::ActiveConnection::HandshakingState*) /proc/self/cwd/./src/core/lib/gprpp/orphanable.h:60:8 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x653b1)
    grpc#8 std::unique_ptr::~unique_ptr() /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:292:4 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x53ddf)
    grpc#9 grpc_core::Chttp2ServerListener::ActiveConnection::Orphan() /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:581:1 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x487db)
    grpc#10 void grpc_core::OrphanableDelete::operator()(grpc_core::Chttp2ServerListener::ActiveConnection*) /proc/self/cwd/./src/core/lib/gprpp/orphanable.h:60:8 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x66b01)
    grpc#11 std::unique_ptr::~unique_ptr() /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:292:4 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x53d1f)
    grpc#12 std::pair>::~pair() /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_pair.h:208:12 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x597f9)
    grpc#13 void __gnu_cxx::new_allocator>>>::destroy>>(std::pair>*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/ext/new_allocator.h:152:10 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x597c1)
    grpc#14 void std::allocator_traits>>>>::destroy>>(std::allocator>>>&, std::pair>*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/alloc_traits.h:496:8 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x59725)
    grpc#15 std::_Rb_tree>, std::_Select1st>>, std::less, std::allocator>>>::_M_destroy_node(std::_Rb_tree_node>>*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:642:2 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x59674)
    grpc#16 std::_Rb_tree>, std::_Select1st>>, std::less, std::allocator>>>::_M_drop_node(std::_Rb_tree_node>>*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:650:2 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x595f9)
    grpc#17 std::_Rb_tree>, std::_Select1st>>, std::less, std::allocator>>>::_M_erase(std::_Rb_tree_node>>*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:1920:4 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x5945f)
    grpc#18 std::_Rb_tree>, std::_Select1st>>, std::less, std::allocator>>>::~_Rb_tree() /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_tree.h:1000:9 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x593c5)
    grpc#19 std::map, std::less, std::allocator>>>::~map() /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/stl_map.h:300:22 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x51525)
    grpc#20 grpc_core::Chttp2ServerListener::Orphan() /proc/self/cwd/src/core/ext/transport/chttp2/server/chttp2_server.cc:923:1 (libsrc_Score_Slibgrpc_Utransport_Uchttp2_Userver.so+0x4b5dd)
    grpc#21 void grpc_core::OrphanableDelete::operator()(grpc_core::Server::ListenerInterface*) /proc/self/cwd/./src/core/lib/gprpp/orphanable.h:60:8 (libsrc_Score_Slibchaotic_Ugood_Userver.so+0x1d0981)
    grpc#22 std::unique_ptr::reset(grpc_core::Server::ListenerInterface*) /usr/lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/unique_ptr.h:402:4 (liblibserver.so+0x1d5c21)
    grpc#23 grpc_core::Server::StopListening() /proc/self/cwd/src/core/server/server.cc:1211:23 (liblibserver.so+0x1b9c62)
    grpc#24 grpc_core::Server::ShutdownAndNotify(grpc_completion_queue*, void*) /proc/self/cwd/src/core/server/server.cc:1195:3 (liblibserver.so+0x1b97da)
    grpc#25 grpc_server_shutdown_and_notify /proc/self/cwd/src/core/server/server.cc:1829:37 (liblibserver.so+0x1bf212)
    grpc#26 grpc_core::CoreEnd2endTest::ShutdownServerAndNotify(int) /proc/self/cwd/./test/core/end2end/end2end_tests.h:459:5 (libtest_Score_Send2end_Slibconnectivity_Ulibrary.so+0x33370)
    grpc#27 grpc_core::(anonymous namespace)::CoreEnd2endTest_RetryHttp2Test_ConnectivityWatch::RunTest() /proc/self/cwd/test/core/end2end/tests/connectivity.cc:74:3 (libtest_Score_Send2end_Slibconnectivity_Ulibrary.so+0x2ee8d)
    grpc#28 grpc_core::(anonymous namespace)::CoreEnd2endTest_RetryHttp2Test_ConnectivityWatch::TestBody() /proc/self/cwd/test/core/end2end/tests/connectivity.cc:32:1 (libtest_Score_Send2end_Slibconnectivity_Ulibrary.so+0x2dc96)
    grpc#29 void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2612:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x16d2dc)
    grpc#30 void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2648:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x14a51d)
    grpc#31 testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2687:5 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x120458)
    grpc#32 testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2836:11 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x1215f3)
    grpc#33 testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:3015:30 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x12230c)
    grpc#34 testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5921:44 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x138142)
    grpc#35 bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2612:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x17508f)
    grpc#36 bool testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2648:14 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x14e5b3)
    grpc#37 testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5485:10 (libexternal_Scom_Ugoogle_Ugoogletest_Slibgtest.so+0x13795b)
    grpc#38 RUN_ALL_TESTS() /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2316:73 (libtest_Score_Send2end_Slibend2end_Utest_Umain.so+0x8457)
    grpc#39 main /proc/self/cwd/test/core/end2end/end2end_test_main.cc:50:10 (libtest_Score_Send2end_Slibend2end_Utest_Umain.so+0x77b6)
```

We start the connection outside the critical region and that's where we supply the listener ref to the connection. There is a freak case where the connection can be orphaned due to the listener stopping to serve and the `Orphan()` would also be trying to access the listener ref resulting in a race.

Closes grpc#37683

COPYBARA_INTEGRATE_REVIEW=grpc#37683 from yashykt:FixChttp2ServerRace e3c4529
PiperOrigin-RevId: 681552145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant