PR comments, round 3

reviewable/pr13932/r5
David Garcia Quintas 7 years ago
parent dfa2851462
commit be1b7f986e
  1. 6
      src/core/ext/filters/client_channel/client_channel.cc
  2. 8
      src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc
  3. 4
      src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
  4. 2
      src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
  5. 9
      src/core/ext/filters/client_channel/subchannel.cc
  6. 9
      src/core/lib/support/ref_counted_ptr.h

@ -1031,7 +1031,7 @@ static void create_subchannel_call_locked(grpc_call_element* elem,
static void pick_done_locked(grpc_call_element* elem, grpc_error* error) {
call_data* calld = (call_data*)elem->call_data;
channel_data* chand = (channel_data*)elem->channel_data;
if (!calld->pick.connected_subchannel) {
if (calld->pick.connected_subchannel == nullptr) {
// Failed to create subchannel.
GRPC_ERROR_UNREF(calld->error);
calld->error = error == GRPC_ERROR_NONE
@ -1283,7 +1283,7 @@ static void start_pick_locked(void* arg, grpc_error* ignored) {
grpc_call_element* elem = (grpc_call_element*)arg;
call_data* calld = (call_data*)elem->call_data;
channel_data* chand = (channel_data*)elem->channel_data;
GPR_ASSERT(!calld->pick.connected_subchannel);
GPR_ASSERT(calld->pick.connected_subchannel == nullptr);
if (chand->lb_policy != nullptr) {
// We already have an LB policy, so ask it for a pick.
if (pick_callback_start_locked(elem)) {
@ -1462,7 +1462,7 @@ static void cc_destroy_call_elem(grpc_call_element* elem,
"client_channel_destroy_call");
}
GPR_ASSERT(calld->waiting_for_pick_batches_count == 0);
if (calld->pick.connected_subchannel) {
if (calld->pick.connected_subchannel != nullptr) {
calld->pick.connected_subchannel.reset();
}
for (size_t i = 0; i < GRPC_CONTEXT_COUNT; ++i) {

@ -322,7 +322,7 @@ static void pending_pick_set_metadata_and_context(pending_pick* pp) {
/* if connected_subchannel is nullptr, no pick has been made by the RR
* policy (e.g., all addresses failed to connect). There won't be any
* user_data/token available */
if (pp->pick->connected_subchannel.get() != nullptr) {
if (pp->pick->connected_subchannel != nullptr) {
if (!GRPC_MDISNULL(pp->lb_token)) {
initial_metadata_add_lb_token(pp->pick->initial_metadata,
&pp->pick->lb_token_mdelem_storage,
@ -935,8 +935,7 @@ static void glb_shutdown_locked(grpc_lb_policy* pol,
}
gpr_free(pp);
} else {
pp->pick->connected_subchannel =
grpc_core::MakeRefCounted<grpc_core::ConnectedSubchannel>(nullptr);
pp->pick->connected_subchannel.reset(nullptr);
GRPC_CLOSURE_SCHED(&pp->on_complete, GRPC_ERROR_REF(error));
}
pp = next;
@ -973,8 +972,7 @@ static void glb_cancel_pick_locked(grpc_lb_policy* pol,
while (pp != nullptr) {
pending_pick* next = pp->next;
if (pp->pick == pick) {
pick->connected_subchannel =
grpc_core::MakeRefCounted<grpc_core::ConnectedSubchannel>(nullptr);
pick->connected_subchannel.reset(nullptr);
GRPC_CLOSURE_SCHED(&pp->on_complete,
GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING(
"Pick Cancelled", &error, 1));

@ -294,7 +294,7 @@ static void pf_update_locked(grpc_lb_policy* policy,
p, p->selected->subchannel, i,
subchannel_list->num_subchannels);
}
if (p->selected->connected_subchannel) {
if (p->selected->connected_subchannel != nullptr) {
sd->connected_subchannel = p->selected->connected_subchannel;
}
p->selected = sd;
@ -458,8 +458,6 @@ static void pf_connectivity_changed_locked(void* arg, grpc_error* error) {
// Cases 1 and 2.
grpc_connectivity_state_set(&p->state_tracker, GRPC_CHANNEL_READY,
GRPC_ERROR_NONE, "connecting_ready");
sd->connected_subchannel =
grpc_subchannel_get_connected_subchannel(sd->subchannel);
p->selected = sd;
if (grpc_lb_pick_first_trace.enabled()) {
gpr_log(GPR_INFO, "Pick First %p selected subchannel %p", (void*)p,

@ -423,7 +423,7 @@ static void rr_connectivity_changed_locked(void* arg, grpc_error* error) {
break;
}
case GRPC_CHANNEL_READY: {
if (!sd->connected_subchannel) {
if (sd->connected_subchannel == nullptr) {
sd->connected_subchannel =
grpc_subchannel_get_connected_subchannel(sd->subchannel);
}

@ -56,10 +56,6 @@
#define GRPC_SUBCHANNEL_RECONNECT_MAX_BACKOFF_SECONDS 120
#define GRPC_SUBCHANNEL_RECONNECT_JITTER 0.2
#define GET_CONNECTED_SUBCHANNEL(subchannel, barrier) \
((grpc_core::ConnectedSubchannel*)(gpr_atm_##barrier##_load( \
&(subchannel)->connected_subchannel)))
namespace {
struct state_watcher {
grpc_closure closure;
@ -443,7 +439,7 @@ static void maybe_start_connecting_locked(grpc_subchannel* c) {
return;
}
if (c->connected_subchannel) {
if (c->connected_subchannel != nullptr) {
/* Already connected: don't restart */
return;
}
@ -524,7 +520,7 @@ static void on_connected_subchannel_connectivity_changed(void* p,
switch (connected_subchannel_watcher->connectivity_state) {
case GRPC_CHANNEL_TRANSIENT_FAILURE:
case GRPC_CHANNEL_SHUTDOWN: {
if (!c->disconnected && c->connected_subchannel) {
if (!c->disconnected && c->connected_subchannel != nullptr) {
if (grpc_trace_stream_refcount.enabled()) {
gpr_log(GPR_INFO,
"Connected subchannel %p of subchannel %p has gone into %s. "
@ -605,7 +601,6 @@ static bool publish_transport_locked(grpc_subchannel* c) {
/* publish */
c->connected_subchannel.reset(
grpc_core::New<grpc_core::ConnectedSubchannel>(stk));
gpr_atm_full_barrier();
gpr_log(GPR_INFO, "New connected subchannel at %p for subchannel %p",
c->connected_subchannel.get(), c);

@ -76,7 +76,14 @@ class RefCountedPtr {
T& operator*() const { return *value_; }
T* operator->() const { return value_; }
explicit operator bool() const { return get() != nullptr; }
bool operator==(const RefCountedPtr& other) const {
return value_ == other.value_;
}
bool operator==(T* other) const { return value_ == other; }
bool operator!=(const RefCountedPtr& other) const {
return value_ != other.value_;
}
bool operator!=(T* other) const { return value_ != other; }
private:
T* value_ = nullptr;

Loading…
Cancel
Save