Fix thread-analaysis warnings on client_channel (#28744)

pull/28687/head^2
Esun Kim 3 years ago committed by GitHub
parent 436bd933f3
commit 781100f765
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 58
      src/core/ext/filters/client_channel/client_channel.cc
  2. 50
      src/core/ext/filters/client_channel/client_channel.h

@ -417,7 +417,7 @@ class ClientChannel::ResolverResultHandler : public Resolver::ResultHandler {
}
void ReportResult(Resolver::Result result) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
chand_->OnResolverResultChangedLocked(std::move(result));
}
@ -499,7 +499,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
void WatchConnectivityState(
grpc_connectivity_state initial_state,
std::unique_ptr<ConnectivityStateWatcherInterface> watcher) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
auto& watcher_wrapper = watcher_map_[watcher.get()];
GPR_ASSERT(watcher_wrapper == nullptr);
watcher_wrapper = new WatcherWrapper(std::move(watcher),
@ -512,7 +512,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
}
void CancelConnectivityStateWatch(ConnectivityStateWatcherInterface* watcher)
override ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
override ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
auto it = watcher_map_.find(watcher);
GPR_ASSERT(it != watcher_map_.end());
subchannel_->CancelConnectivityStateWatch(health_check_service_name_,
@ -565,10 +565,10 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
~WatcherWrapper() override {
auto* parent = parent_.release(); // ref owned by lambda
parent->chand_->work_serializer_->Run(
[parent]()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(parent_->chand_->work_serializer_) {
parent->Unref(DEBUG_LOCATION, "WatcherWrapper");
},
[parent]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(
*parent_->chand_->work_serializer_) {
parent->Unref(DEBUG_LOCATION, "WatcherWrapper");
},
DEBUG_LOCATION);
}
@ -581,11 +581,11 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
}
Ref().release(); // ref owned by lambda
parent_->chand_->work_serializer_->Run(
[this]()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(parent_->chand_->work_serializer_) {
ApplyUpdateInControlPlaneWorkSerializer();
Unref();
},
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(
*parent_->chand_->work_serializer_) {
ApplyUpdateInControlPlaneWorkSerializer();
Unref();
},
DEBUG_LOCATION);
}
@ -607,7 +607,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
private:
void ApplyUpdateInControlPlaneWorkSerializer()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(parent_->chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*parent_->chand_->work_serializer_) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) {
gpr_log(GPR_INFO,
"chand=%p: processing connectivity change in work serializer "
@ -667,7 +667,7 @@ class ClientChannel::SubchannelWrapper : public SubchannelInterface {
// CancelConnectivityStateWatch() with its watcher, we know the
// corresponding WrapperWatcher to cancel on the underlying subchannel.
std::map<ConnectivityStateWatcherInterface*, WatcherWrapper*> watcher_map_
ABSL_GUARDED_BY(&ClientChannel::work_serializer_);
ABSL_GUARDED_BY(*chand_->work_serializer_);
};
//
@ -697,7 +697,7 @@ ClientChannel::ExternalConnectivityWatcher::ExternalConnectivityWatcher(
}
// Pass the ref from creating the object to Start().
chand_->work_serializer_->Run(
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
// The ref is passed to AddWatcherLocked().
AddWatcherLocked();
},
@ -747,7 +747,7 @@ void ClientChannel::ExternalConnectivityWatcher::Notify(
// automatically remove all watchers in that case.
if (state != GRPC_CHANNEL_SHUTDOWN) {
chand_->work_serializer_->Run(
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
RemoveWatcherLocked();
},
DEBUG_LOCATION);
@ -763,7 +763,7 @@ void ClientChannel::ExternalConnectivityWatcher::Cancel() {
ExecCtx::Run(DEBUG_LOCATION, on_complete_, GRPC_ERROR_CANCELLED);
// Hop back into the work_serializer to clean up.
chand_->work_serializer_->Run(
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
RemoveWatcherLocked();
},
DEBUG_LOCATION);
@ -794,7 +794,7 @@ class ClientChannel::ConnectivityWatcherAdder {
watcher_(std::move(watcher)) {
GRPC_CHANNEL_STACK_REF(chand_->owning_stack_, "ConnectivityWatcherAdder");
chand_->work_serializer_->Run(
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
AddWatcherLocked();
},
DEBUG_LOCATION);
@ -802,7 +802,7 @@ class ClientChannel::ConnectivityWatcherAdder {
private:
void AddWatcherLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
chand_->state_tracker_.AddWatcher(initial_state_, std::move(watcher_));
GRPC_CHANNEL_STACK_UNREF(chand_->owning_stack_, "ConnectivityWatcherAdder");
delete this;
@ -824,7 +824,7 @@ class ClientChannel::ConnectivityWatcherRemover {
: chand_(chand), watcher_(watcher) {
GRPC_CHANNEL_STACK_REF(chand_->owning_stack_, "ConnectivityWatcherRemover");
chand_->work_serializer_->Run(
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
[this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
RemoveWatcherLocked();
},
DEBUG_LOCATION);
@ -832,7 +832,7 @@ class ClientChannel::ConnectivityWatcherRemover {
private:
void RemoveWatcherLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
chand_->state_tracker_.RemoveWatcher(watcher_);
GRPC_CHANNEL_STACK_UNREF(chand_->owning_stack_,
"ConnectivityWatcherRemover");
@ -861,7 +861,7 @@ class ClientChannel::ClientChannelControlHelper
RefCountedPtr<SubchannelInterface> CreateSubchannel(
ServerAddress address, const grpc_channel_args& args) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
if (chand_->resolver_ == nullptr) return nullptr; // Shutting down.
// Determine health check service name.
absl::optional<std::string> health_check_service_name;
@ -932,7 +932,7 @@ class ClientChannel::ClientChannelControlHelper
void UpdateState(
grpc_connectivity_state state, const absl::Status& status,
std::unique_ptr<LoadBalancingPolicy::SubchannelPicker> picker) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
if (chand_->resolver_ == nullptr) return; // Shutting down.
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) {
const char* extra = chand_->disconnect_error_ == GRPC_ERROR_NONE
@ -950,7 +950,7 @@ class ClientChannel::ClientChannelControlHelper
}
void RequestReresolution() override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
if (chand_->resolver_ == nullptr) return; // Shutting down.
if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_routing_trace)) {
gpr_log(GPR_INFO, "chand=%p: started name re-resolving", chand_);
@ -963,7 +963,7 @@ class ClientChannel::ClientChannelControlHelper
}
void AddTraceEvent(TraceSeverity severity, absl::string_view message) override
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_) {
if (chand_->resolver_ == nullptr) return; // Shutting down.
if (chand_->channelz_node_ != nullptr) {
chand_->channelz_node_->AddTraceEvent(
@ -1663,7 +1663,7 @@ grpc_error_handle ClientChannel::DoPingLocked(grpc_transport_op* op) {
&result,
// Complete pick.
[op](LoadBalancingPolicy::PickResult::Complete* complete_pick)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(&ClientChannel::work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*ClientChannel::work_serializer_) {
SubchannelWrapper* subchannel = static_cast<SubchannelWrapper*>(
complete_pick->subchannel.get());
RefCountedPtr<ConnectedSubchannel> connected_subchannel =
@ -1755,7 +1755,7 @@ void ClientChannel::StartTransportOp(grpc_channel_element* elem,
// Pop into control plane work_serializer for remaining ops.
GRPC_CHANNEL_STACK_REF(chand->owning_stack_, "start_transport_op");
chand->work_serializer_->Run(
[chand, op]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand->work_serializer_) {
[chand, op]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand->work_serializer_) {
chand->StartTransportOpLocked(op);
},
DEBUG_LOCATION);
@ -1817,7 +1817,7 @@ grpc_connectivity_state ClientChannel::CheckConnectivityState(
if (out == GRPC_CHANNEL_IDLE && try_to_connect) {
GRPC_CHANNEL_STACK_REF(owning_stack_, "TryToConnect");
work_serializer_->Run([this]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(
work_serializer_) { TryToConnectLocked(); },
*work_serializer_) { TryToConnectLocked(); },
DEBUG_LOCATION);
}
return out;
@ -2323,7 +2323,7 @@ bool ClientChannel::CallData::CheckResolutionLocked(grpc_call_element* elem,
auto* chand = static_cast<ClientChannel*>(arg);
chand->work_serializer_->Run(
[chand]()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand->work_serializer_) {
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand->work_serializer_) {
chand->CheckConnectivityState(/*try_to_connect=*/true);
GRPC_CHANNEL_STACK_UNREF(chand->owning_stack_,
"CheckResolutionLocked");

@ -177,9 +177,9 @@ class ClientChannel {
// Adds the watcher to state_tracker_. Consumes the ref that is passed to it
// from Start().
void AddWatcherLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_);
void RemoveWatcherLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(chand_->work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*chand_->work_serializer_);
ClientChannel* chand_;
grpc_polling_entity pollent_;
@ -215,43 +215,43 @@ class ClientChannel {
// work_serializer_.
void OnResolverResultChangedLocked(Resolver::Result result)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void OnResolverErrorLocked(absl::Status status)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void CreateOrUpdateLbPolicyLocked(
RefCountedPtr<LoadBalancingPolicy::Config> lb_policy_config,
const absl::optional<std::string>& health_check_service_name,
Resolver::Result result) ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
Resolver::Result result) ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
OrphanablePtr<LoadBalancingPolicy> CreateLbPolicyLocked(
const grpc_channel_args& args)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void UpdateStateAndPickerLocked(
grpc_connectivity_state state, const absl::Status& status,
const char* reason,
std::unique_ptr<LoadBalancingPolicy::SubchannelPicker> picker)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void UpdateServiceConfigInControlPlaneLocked(
RefCountedPtr<ServiceConfig> service_config,
RefCountedPtr<ConfigSelector> config_selector, const char* lb_policy_name)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void UpdateServiceConfigInDataPlaneLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void CreateResolverLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
void CreateResolverLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void DestroyResolverAndLbPolicyLocked()
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
grpc_error_handle DoPingLocked(grpc_transport_op* op)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void StartTransportOpLocked(grpc_transport_op* op)
ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
void TryToConnectLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(work_serializer_);
void TryToConnectLocked() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*work_serializer_);
// These methods all require holding resolution_mu_.
void AddResolverQueuedCall(ResolverQueuedCall* call,
@ -310,28 +310,28 @@ class ClientChannel {
// Fields used in the control plane. Guarded by work_serializer.
//
std::shared_ptr<WorkSerializer> work_serializer_;
ConnectivityStateTracker state_tracker_ ABSL_GUARDED_BY(work_serializer_);
OrphanablePtr<Resolver> resolver_ ABSL_GUARDED_BY(work_serializer_);
ConnectivityStateTracker state_tracker_ ABSL_GUARDED_BY(*work_serializer_);
OrphanablePtr<Resolver> resolver_ ABSL_GUARDED_BY(*work_serializer_);
bool previous_resolution_contained_addresses_
ABSL_GUARDED_BY(work_serializer_) = false;
ABSL_GUARDED_BY(*work_serializer_) = false;
RefCountedPtr<ServiceConfig> saved_service_config_
ABSL_GUARDED_BY(work_serializer_);
ABSL_GUARDED_BY(*work_serializer_);
RefCountedPtr<ConfigSelector> saved_config_selector_
ABSL_GUARDED_BY(work_serializer_);
ABSL_GUARDED_BY(*work_serializer_);
OrphanablePtr<LoadBalancingPolicy> lb_policy_
ABSL_GUARDED_BY(work_serializer_);
ABSL_GUARDED_BY(*work_serializer_);
RefCountedPtr<SubchannelPoolInterface> subchannel_pool_
ABSL_GUARDED_BY(work_serializer_);
ABSL_GUARDED_BY(*work_serializer_);
// The number of SubchannelWrapper instances referencing a given Subchannel.
std::map<Subchannel*, int> subchannel_refcount_map_
ABSL_GUARDED_BY(work_serializer_);
ABSL_GUARDED_BY(*work_serializer_);
// The set of SubchannelWrappers that currently exist.
// No need to hold a ref, since the map is updated in the control-plane
// work_serializer when the SubchannelWrappers are created and destroyed.
std::set<SubchannelWrapper*> subchannel_wrappers_
ABSL_GUARDED_BY(work_serializer_);
int keepalive_time_ ABSL_GUARDED_BY(work_serializer_) = -1;
grpc_error_handle disconnect_error_ ABSL_GUARDED_BY(work_serializer_) =
ABSL_GUARDED_BY(*work_serializer_);
int keepalive_time_ ABSL_GUARDED_BY(*work_serializer_) = -1;
grpc_error_handle disconnect_error_ ABSL_GUARDED_BY(*work_serializer_) =
GRPC_ERROR_NONE;
//

Loading…
Cancel
Save