Reviewer comments

pull/23418/head
Yash Tibrewal 5 years ago
parent e03b0b6deb
commit 01f992f9d6
  1. 86
      src/core/ext/filters/client_channel/client_channel.cc

@ -171,22 +171,14 @@ class ChannelData {
grpc_connectivity_state* state,
grpc_closure* on_complete,
grpc_closure* watcher_timer_init) {
auto watcher = new ExternalConnectivityWatcher(
this, pollent, state, on_complete, watcher_timer_init);
{
MutexLock lock(&external_watchers_mu_);
// Store a ref to the watcher in the external_watchers_ map.
watcher->AddWatcherToExternalWatchersMapLocked(&external_watchers_,
on_complete);
}
// Pass the ref from creating the object to Start().
watcher->Start();
new ExternalConnectivityWatcher(this, pollent, state, on_complete,
watcher_timer_init);
}
void RemoveExternalConnectivityWatcher(grpc_closure* on_complete,
bool cancel) {
ExternalConnectivityWatcher::RemoveWatcherFromExternalWatchersMap(
&external_watchers_mu_, &external_watchers_, on_complete, cancel);
this, on_complete, cancel);
}
int NumExternalConnectivityWatchers() const {
@ -217,23 +209,10 @@ class ChannelData {
~ExternalConnectivityWatcher();
// Adds the watcher to the external_watchers_ map. Synchronized by
// external_watchers_mu_
void AddWatcherToExternalWatchersMapLocked(
std::map<grpc_closure*, RefCountedPtr<ExternalConnectivityWatcher>>*
external_watchers,
grpc_closure* on_complete);
// Removes the watcher from the external_watchers_ map.
static void RemoveWatcherFromExternalWatchersMap(
Mutex* external_watchers_mu,
std::map<grpc_closure*, RefCountedPtr<ExternalConnectivityWatcher>>*
external_watchers,
grpc_closure* on_complete, bool cancel);
// Starts the watch. Consumes the ref from the creation of the
// ExternalConnectivityWatcher object.
void Start();
static void RemoveWatcherFromExternalWatchersMap(ChannelData* chand,
grpc_closure* on_complete,
bool cancel);
void Notify(grpc_connectivity_state state) override;
@ -1181,6 +1160,21 @@ ChannelData::ExternalConnectivityWatcher::ExternalConnectivityWatcher(
grpc_polling_entity_add_to_pollset_set(&pollent_,
chand_->interested_parties_);
GRPC_CHANNEL_STACK_REF(chand_->owning_stack_, "ExternalConnectivityWatcher");
{
MutexLock lock(&chand_->external_watchers_mu_);
// Will be deleted when the watch is complete.
GPR_ASSERT(chand->external_watchers_[on_complete] == nullptr);
// Store a ref to the watcher in the external_watchers_ map.
chand->external_watchers_[on_complete] =
Ref(DEBUG_LOCATION, "AddWatcherToExternalWatchersMapLocked");
}
// Pass the ref from creating the object to Start().
chand_->work_serializer_->Run(
[this]() {
// The ref is passed to AddWatcherLocked().
AddWatcherLocked();
},
DEBUG_LOCATION);
}
ChannelData::ExternalConnectivityWatcher::~ExternalConnectivityWatcher() {
@ -1191,29 +1185,16 @@ ChannelData::ExternalConnectivityWatcher::~ExternalConnectivityWatcher() {
}
void ChannelData::ExternalConnectivityWatcher::
AddWatcherToExternalWatchersMapLocked(
std::map<grpc_closure*, RefCountedPtr<ExternalConnectivityWatcher>>*
external_watchers,
grpc_closure* on_complete) {
// Will be deleted when the watch is complete.
GPR_ASSERT((*external_watchers)[on_complete] == nullptr);
(*external_watchers)[on_complete] =
Ref(DEBUG_LOCATION, "AddWatcherToExternalWatchersMapLocked");
}
void ChannelData::ExternalConnectivityWatcher::
RemoveWatcherFromExternalWatchersMap(
Mutex* external_watchers_mu,
std::map<grpc_closure*, RefCountedPtr<ExternalConnectivityWatcher>>*
external_watchers,
grpc_closure* on_complete, bool cancel) {
RemoveWatcherFromExternalWatchersMap(ChannelData* chand,
grpc_closure* on_complete,
bool cancel) {
RefCountedPtr<ExternalConnectivityWatcher> watcher;
{
MutexLock lock(external_watchers_mu);
auto it = (*external_watchers).find(on_complete);
if (it != (*external_watchers).end()) {
MutexLock lock(&chand->external_watchers_mu_);
auto it = chand->external_watchers_.find(on_complete);
if (it != chand->external_watchers_.end()) {
watcher = std::move(it->second);
(*external_watchers).erase(it);
chand->external_watchers_.erase(it);
}
}
// watcher->Cancel() will hop into the WorkSerializer, so we have to unlock
@ -1221,17 +1202,6 @@ void ChannelData::ExternalConnectivityWatcher::
if (watcher != nullptr && cancel) watcher->Cancel();
}
void ChannelData::ExternalConnectivityWatcher::Start() {
// No need to take a ref since Start() consumes the ref from the
// creation of the object.
chand_->work_serializer_->Run(
[this]() {
// The ref is passed to AddWatcherLocked().
AddWatcherLocked();
},
DEBUG_LOCATION);
}
void ChannelData::ExternalConnectivityWatcher::Notify(
grpc_connectivity_state state) {
bool done = false;

Loading…
Cancel
Save