diff options
Diffstat (limited to 'src')
43 files changed, 240 insertions, 80 deletions
diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 3813190794..fa0c280f80 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -3242,7 +3242,7 @@ static void on_external_watch_complete_locked(void* arg, grpc_error* error) { "external_connectivity_watcher"); external_connectivity_watcher_list_remove(w->chand, w); gpr_free(w); - GRPC_CLOSURE_RUN(follow_up, GRPC_ERROR_REF(error)); + GRPC_CLOSURE_SCHED(follow_up, GRPC_ERROR_REF(error)); } static void watch_connectivity_state_locked(void* arg, diff --git a/src/core/ext/filters/client_channel/lb_policy.h b/src/core/ext/filters/client_channel/lb_policy.h index 454e00a690..dab4466b21 100644 --- a/src/core/ext/filters/client_channel/lb_policy.h +++ b/src/core/ext/filters/client_channel/lb_policy.h @@ -162,9 +162,7 @@ class LoadBalancingPolicy GRPC_ABSTRACT_BASE_CLASS protected: - // So Delete() can access our protected dtor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE explicit LoadBalancingPolicy(const Args& args); virtual ~LoadBalancingPolicy(); diff --git a/src/core/ext/filters/client_channel/resolver.h b/src/core/ext/filters/client_channel/resolver.h index 02380314dd..c7e37e4468 100644 --- a/src/core/ext/filters/client_channel/resolver.h +++ b/src/core/ext/filters/client_channel/resolver.h @@ -105,9 +105,7 @@ class Resolver : public InternallyRefCountedWithTracing<Resolver> { GRPC_ABSTRACT_BASE_CLASS protected: - // So Delete() can access our protected dtor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE /// Does NOT take ownership of the reference to \a combiner. // TODO(roth): Once we have a C++-like interface for combiners, this diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index 140441da10..ad6b6dd192 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -408,7 +408,7 @@ static void on_external_state_watcher_done(void* arg, grpc_error* error) { gpr_mu_unlock(&w->subchannel->mu); GRPC_SUBCHANNEL_WEAK_UNREF(w->subchannel, "external_state_watcher"); gpr_free(w); - GRPC_CLOSURE_RUN(follow_up, GRPC_ERROR_REF(error)); + GRPC_CLOSURE_SCHED(follow_up, GRPC_ERROR_REF(error)); } static void on_alarm(void* arg, grpc_error* error) { diff --git a/src/core/ext/filters/http/client_authority_filter.cc b/src/core/ext/filters/http/client_authority_filter.cc index 1f57ab5ce6..63b9150aec 100644 --- a/src/core/ext/filters/http/client_authority_filter.cc +++ b/src/core/ext/filters/http/client_authority_filter.cc @@ -59,8 +59,9 @@ void authority_start_transport_stream_op_batch( initial_metadata->idx.named.authority == nullptr) { grpc_error* error = grpc_metadata_batch_add_head( initial_metadata, &calld->authority_storage, - grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY, - grpc_slice_ref(chand->default_authority))); + grpc_mdelem_from_slices( + GRPC_MDSTR_AUTHORITY, + grpc_slice_ref_internal(chand->default_authority))); if (error != GRPC_ERROR_NONE) { grpc_transport_stream_op_batch_finish_with_failure(batch, error, calld->call_combiner); @@ -110,7 +111,7 @@ grpc_error* init_channel_elem(grpc_channel_element* elem, /* Destructor for channel data */ void destroy_channel_elem(grpc_channel_element* elem) { channel_data* chand = static_cast<channel_data*>(elem->channel_data); - grpc_slice_unref(chand->default_authority); + grpc_slice_unref_internal(chand->default_authority); } } // namespace diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 7ff7cabfbd..cc4a823798 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1684,16 +1684,16 @@ static void send_ping_locked(grpc_chttp2_transport* t, */ static void send_keepalive_ping_locked(grpc_chttp2_transport* t) { if (t->closed_with_error != GRPC_ERROR_NONE) { - GRPC_CLOSURE_SCHED(&t->start_keepalive_ping_locked, - GRPC_ERROR_REF(t->closed_with_error)); - GRPC_CLOSURE_SCHED(&t->finish_keepalive_ping_locked, - GRPC_ERROR_REF(t->closed_with_error)); + GRPC_CLOSURE_RUN(&t->start_keepalive_ping_locked, + GRPC_ERROR_REF(t->closed_with_error)); + GRPC_CLOSURE_RUN(&t->finish_keepalive_ping_locked, + GRPC_ERROR_REF(t->closed_with_error)); return; } grpc_chttp2_ping_queue* pq = &t->ping_queue; if (!grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_INFLIGHT])) { /* There is a ping in flight. Add yourself to the inflight closure list. */ - GRPC_CLOSURE_SCHED(&t->start_keepalive_ping_locked, GRPC_ERROR_NONE); + GRPC_CLOSURE_RUN(&t->start_keepalive_ping_locked, GRPC_ERROR_NONE); grpc_closure_list_append(&pq->lists[GRPC_CHTTP2_PCL_INFLIGHT], &t->finish_keepalive_ping_locked, GRPC_ERROR_NONE); return; diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 0b88ff7afe..420c2d13e1 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -736,7 +736,7 @@ static void convert_metadata_to_cronet_headers( if (grpc_is_binary_header(GRPC_MDKEY(mdelem))) { grpc_slice wire_value = grpc_chttp2_base64_encode(GRPC_MDVALUE(mdelem)); value = grpc_slice_to_c_string(wire_value); - grpc_slice_unref(wire_value); + grpc_slice_unref_internal(wire_value); } else { value = grpc_slice_to_c_string(GRPC_MDVALUE(mdelem)); } diff --git a/src/core/lib/channel/channel_stack.cc b/src/core/lib/channel/channel_stack.cc index a9459b150d..ef6482cb7f 100644 --- a/src/core/lib/channel/channel_stack.cc +++ b/src/core/lib/channel/channel_stack.cc @@ -193,18 +193,13 @@ void grpc_call_stack_set_pollset_or_pollset_set(grpc_call_stack* call_stack, grpc_polling_entity* pollent) { size_t count = call_stack->count; grpc_call_element* call_elems; - char* user_data; size_t i; call_elems = CALL_ELEMS_FROM_STACK(call_stack); - user_data = (reinterpret_cast<char*>(call_elems)) + - ROUND_UP_TO_ALIGNMENT_SIZE(count * sizeof(grpc_call_element)); /* init per-filter data */ for (i = 0; i < count; i++) { call_elems[i].filter->set_pollset_or_pollset_set(&call_elems[i], pollent); - user_data += - ROUND_UP_TO_ALIGNMENT_SIZE(call_elems[i].filter->sizeof_call_data); } } diff --git a/src/core/lib/channel/channel_stack_builder.cc b/src/core/lib/channel/channel_stack_builder.cc index 8a72449034..df5a783631 100644 --- a/src/core/lib/channel/channel_stack_builder.cc +++ b/src/core/lib/channel/channel_stack_builder.cc @@ -25,9 +25,6 @@ #include <grpc/support/alloc.h> #include <grpc/support/string_util.h> -grpc_core::TraceFlag grpc_trace_channel_stack_builder(false, - "channel_stack_builder"); - typedef struct filter_node { struct filter_node* next; struct filter_node* prev; diff --git a/src/core/lib/channel/channel_stack_builder.h b/src/core/lib/channel/channel_stack_builder.h index c9a170bc88..9196de9378 100644 --- a/src/core/lib/channel/channel_stack_builder.h +++ b/src/core/lib/channel/channel_stack_builder.h @@ -155,6 +155,4 @@ grpc_error* grpc_channel_stack_builder_finish( /// Destroy the builder without creating a channel stack void grpc_channel_stack_builder_destroy(grpc_channel_stack_builder* builder); -extern grpc_core::TraceFlag grpc_trace_channel_stack_builder; - #endif /* GRPC_CORE_LIB_CHANNEL_CHANNEL_STACK_BUILDER_H */ diff --git a/src/core/lib/channel/handshaker.cc b/src/core/lib/channel/handshaker.cc index 2faeb64cb6..86f8699e04 100644 --- a/src/core/lib/channel/handshaker.cc +++ b/src/core/lib/channel/handshaker.cc @@ -28,6 +28,7 @@ #include "src/core/lib/channel/handshaker.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/timer.h" +#include "src/core/lib/slice/slice_internal.h" grpc_core::TraceFlag grpc_handshaker_trace(false, "handshaker"); @@ -220,8 +221,26 @@ static bool call_next_handshaker_locked(grpc_handshake_manager* mgr, // callback. Otherwise, call the next handshaker. if (error != GRPC_ERROR_NONE || mgr->shutdown || mgr->args.exit_early || mgr->index == mgr->count) { + if (error == GRPC_ERROR_NONE && mgr->shutdown) { + error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("handshaker shutdown"); + // TODO(roth): It is currently necessary to shutdown endpoints + // before destroying then, even when we know that there are no + // pending read/write callbacks. This should be fixed, at which + // point this can be removed. + grpc_endpoint_shutdown(mgr->args.endpoint, GRPC_ERROR_REF(error)); + grpc_endpoint_destroy(mgr->args.endpoint); + mgr->args.endpoint = nullptr; + grpc_channel_args_destroy(mgr->args.args); + mgr->args.args = nullptr; + grpc_slice_buffer_destroy_internal(mgr->args.read_buffer); + gpr_free(mgr->args.read_buffer); + mgr->args.read_buffer = nullptr; + } if (grpc_handshaker_trace.enabled()) { - gpr_log(GPR_INFO, "handshake_manager %p: handshaking complete", mgr); + gpr_log(GPR_INFO, + "handshake_manager %p: handshaking complete -- scheduling " + "on_handshake_done with error=%s", + mgr, grpc_error_string(error)); } // Cancel deadline timer, since we're invoking the on_handshake_done // callback now. diff --git a/src/core/lib/debug/stats_data.cc b/src/core/lib/debug/stats_data.cc index 309ece94bb..f8c27db0a8 100644 --- a/src/core/lib/debug/stats_data.cc +++ b/src/core/lib/debug/stats_data.cc @@ -40,6 +40,8 @@ const char* grpc_stats_counter_name[GRPC_STATS_COUNTER_COUNT] = { "pollset_kick_wakeup_fd", "pollset_kick_wakeup_cv", "pollset_kick_own_thread", + "syscall_epoll_ctl", + "pollset_fd_cache_hits", "histogram_slow_lookups", "syscall_write", "syscall_read", @@ -144,6 +146,9 @@ const char* grpc_stats_counter_doc[GRPC_STATS_COUNTER_COUNT] = { "polling wakeup (only valid for epoll1 right now)", "How many times could a polling wakeup be satisfied by keeping the waking " "thread awake? (only valid for epoll1 right now)", + "Number of epoll_ctl calls made (only valid for epollex right now)", + "Number of epoll_ctl calls skipped because the fd was cached as already " + "being added. (only valid for epollex right now)", "Number of times histogram increments went through the slow (binary " "search) path", "Number of write syscalls (or equivalent - eg sendmsg) made by this " diff --git a/src/core/lib/debug/stats_data.h b/src/core/lib/debug/stats_data.h index 37c548095f..1f3861f494 100644 --- a/src/core/lib/debug/stats_data.h +++ b/src/core/lib/debug/stats_data.h @@ -41,6 +41,8 @@ typedef enum { GRPC_STATS_COUNTER_POLLSET_KICK_WAKEUP_FD, GRPC_STATS_COUNTER_POLLSET_KICK_WAKEUP_CV, GRPC_STATS_COUNTER_POLLSET_KICK_OWN_THREAD, + GRPC_STATS_COUNTER_SYSCALL_EPOLL_CTL, + GRPC_STATS_COUNTER_POLLSET_FD_CACHE_HITS, GRPC_STATS_COUNTER_HISTOGRAM_SLOW_LOOKUPS, GRPC_STATS_COUNTER_SYSCALL_WRITE, GRPC_STATS_COUNTER_SYSCALL_READ, @@ -203,6 +205,10 @@ typedef enum { GRPC_STATS_INC_COUNTER(GRPC_STATS_COUNTER_POLLSET_KICK_WAKEUP_CV) #define GRPC_STATS_INC_POLLSET_KICK_OWN_THREAD() \ GRPC_STATS_INC_COUNTER(GRPC_STATS_COUNTER_POLLSET_KICK_OWN_THREAD) +#define GRPC_STATS_INC_SYSCALL_EPOLL_CTL() \ + GRPC_STATS_INC_COUNTER(GRPC_STATS_COUNTER_SYSCALL_EPOLL_CTL) +#define GRPC_STATS_INC_POLLSET_FD_CACHE_HITS() \ + GRPC_STATS_INC_COUNTER(GRPC_STATS_COUNTER_POLLSET_FD_CACHE_HITS) #define GRPC_STATS_INC_HISTOGRAM_SLOW_LOOKUPS() \ GRPC_STATS_INC_COUNTER(GRPC_STATS_COUNTER_HISTOGRAM_SLOW_LOOKUPS) #define GRPC_STATS_INC_SYSCALL_WRITE() \ @@ -443,6 +449,8 @@ void grpc_stats_inc_server_cqs_checked(int x); #define GRPC_STATS_INC_POLLSET_KICK_WAKEUP_FD() #define GRPC_STATS_INC_POLLSET_KICK_WAKEUP_CV() #define GRPC_STATS_INC_POLLSET_KICK_OWN_THREAD() +#define GRPC_STATS_INC_SYSCALL_EPOLL_CTL() +#define GRPC_STATS_INC_POLLSET_FD_CACHE_HITS() #define GRPC_STATS_INC_HISTOGRAM_SLOW_LOOKUPS() #define GRPC_STATS_INC_SYSCALL_WRITE() #define GRPC_STATS_INC_SYSCALL_READ() diff --git a/src/core/lib/debug/stats_data.yaml b/src/core/lib/debug/stats_data.yaml index af4553028e..775b09df74 100644 --- a/src/core/lib/debug/stats_data.yaml +++ b/src/core/lib/debug/stats_data.yaml @@ -63,6 +63,12 @@ doc: How many times could a polling wakeup be satisfied by keeping the waking thread awake? (only valid for epoll1 right now) +# polling +- counter: syscall_epoll_ctl + doc: Number of epoll_ctl calls made (only valid for epollex right now) +- counter: pollset_fd_cache_hits + doc: Number of epoll_ctl calls skipped because the fd was cached as + already being added. (only valid for epollex right now) # stats system - counter: histogram_slow_lookups doc: Number of times histogram increments went through the slow diff --git a/src/core/lib/debug/stats_data_bq_schema.sql b/src/core/lib/debug/stats_data_bq_schema.sql index 04b6d471f6..7d1ab1dae9 100644 --- a/src/core/lib/debug/stats_data_bq_schema.sql +++ b/src/core/lib/debug/stats_data_bq_schema.sql @@ -12,6 +12,8 @@ pollset_kicked_again_per_iteration:FLOAT, pollset_kick_wakeup_fd_per_iteration:FLOAT, pollset_kick_wakeup_cv_per_iteration:FLOAT, pollset_kick_own_thread_per_iteration:FLOAT, +syscall_epoll_ctl_per_iteration:FLOAT, +pollset_fd_cache_hits_per_iteration:FLOAT, histogram_slow_lookups_per_iteration:FLOAT, syscall_write_per_iteration:FLOAT, syscall_read_per_iteration:FLOAT, diff --git a/src/core/lib/gprpp/memory.h b/src/core/lib/gprpp/memory.h index ba2f546675..1354109bf3 100644 --- a/src/core/lib/gprpp/memory.h +++ b/src/core/lib/gprpp/memory.h @@ -27,6 +27,17 @@ #include <memory> #include <utility> +// Add this to a class that want to use Delete(), but has a private or +// protected destructor. +#define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE \ + template <typename T> \ + friend void Delete(T*); +// Add this to a class that want to use New(), but has a private or +// protected constructor. +#define GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_NEW \ + template <typename T, typename... Args> \ + friend T* New(Args&&...); + namespace grpc_core { // The alignment of memory returned by gpr_malloc(). diff --git a/src/core/lib/gprpp/orphanable.h b/src/core/lib/gprpp/orphanable.h index 73a73995c7..d0ec9b6461 100644 --- a/src/core/lib/gprpp/orphanable.h +++ b/src/core/lib/gprpp/orphanable.h @@ -83,9 +83,7 @@ class InternallyRefCounted : public Orphanable { GRPC_ABSTRACT_BASE_CLASS protected: - // Allow Delete() to access destructor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). friend class RefCountedPtr<Child>; @@ -128,9 +126,7 @@ class InternallyRefCountedWithTracing : public Orphanable { GRPC_ABSTRACT_BASE_CLASS protected: - // Allow Delete() to access destructor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE // Allow RefCountedPtr<> to access Unref() and IncrementRefCount(). friend class RefCountedPtr<Child>; diff --git a/src/core/lib/gprpp/ref_counted.h b/src/core/lib/gprpp/ref_counted.h index c67e3f315c..ddac5bd475 100644 --- a/src/core/lib/gprpp/ref_counted.h +++ b/src/core/lib/gprpp/ref_counted.h @@ -65,9 +65,7 @@ class RefCounted { GRPC_ABSTRACT_BASE_CLASS protected: - // Allow Delete() to access destructor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE RefCounted() { gpr_ref_init(&refs_, 1); } @@ -135,9 +133,7 @@ class RefCountedWithTracing { GRPC_ABSTRACT_BASE_CLASS protected: - // Allow Delete() to access destructor. - template <typename T> - friend void Delete(T*); + GPRC_ALLOW_CLASS_TO_USE_NON_PUBLIC_DELETE RefCountedWithTracing() : RefCountedWithTracing(static_cast<TraceFlag*>(nullptr)) {} diff --git a/src/core/lib/iomgr/combiner.cc b/src/core/lib/iomgr/combiner.cc index 9429842eb8..6789e4d12d 100644 --- a/src/core/lib/iomgr/combiner.cc +++ b/src/core/lib/iomgr/combiner.cc @@ -63,11 +63,12 @@ struct grpc_combiner { gpr_refcount refs; }; +static void combiner_run(grpc_closure* closure, grpc_error* error); static void combiner_exec(grpc_closure* closure, grpc_error* error); static void combiner_finally_exec(grpc_closure* closure, grpc_error* error); static const grpc_closure_scheduler_vtable scheduler = { - combiner_exec, combiner_exec, "combiner:immediately"}; + combiner_run, combiner_exec, "combiner:immediately"}; static const grpc_closure_scheduler_vtable finally_scheduler = { combiner_finally_exec, combiner_finally_exec, "combiner:finally"}; @@ -343,6 +344,22 @@ static void combiner_finally_exec(grpc_closure* closure, grpc_error* error) { grpc_closure_list_append(&lock->final_list, closure, error); } +static void combiner_run(grpc_closure* closure, grpc_error* error) { + grpc_combiner* lock = COMBINER_FROM_CLOSURE_SCHEDULER(closure, scheduler); +#ifndef NDEBUG + closure->scheduled = false; + GRPC_COMBINER_TRACE(gpr_log( + GPR_DEBUG, + "Combiner:%p grpc_combiner_run closure:%p created [%s:%d] run [%s:%d]", + lock, closure, closure->file_created, closure->line_created, + closure->file_initiated, closure->line_initiated)); +#endif + GPR_ASSERT(grpc_core::ExecCtx::Get()->combiner_data()->active_combiner == + lock); + closure->cb(closure->cb_arg, error); + GRPC_ERROR_UNREF(error); +} + static void enqueue_finally(void* closure, grpc_error* error) { combiner_finally_exec(static_cast<grpc_closure*>(closure), GRPC_ERROR_REF(error)); diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc index 98369ddd6e..4c6cff7fe2 100644 --- a/src/core/lib/iomgr/ev_epollex_linux.cc +++ b/src/core/lib/iomgr/ev_epollex_linux.cc @@ -63,6 +63,7 @@ // a keepalive ping timeout issue. We may want to revert https://github // .com/grpc/grpc/pull/14943 once we figure out the root cause. #define MAX_EPOLL_EVENTS_HANDLED_EACH_POLL_CALL 16 +#define MAX_PROBE_EPOLL_FDS 32 grpc_core::DebugOnlyTraceFlag grpc_trace_pollable_refcount(false, "pollable_refcount"); @@ -75,6 +76,12 @@ typedef enum { PO_MULTI, PO_FD, PO_EMPTY } pollable_type; typedef struct pollable pollable; +typedef struct cached_fd { + intptr_t salt; + int fd; + uint64_t last_used; +} cached_fd; + /// A pollable is something that can be polled: it has an epoll set to poll on, /// and a wakeup fd for kicks /// There are three broad types: @@ -103,6 +110,11 @@ struct pollable { int event_cursor; int event_count; struct epoll_event events[MAX_EPOLL_EVENTS]; + + // Maintain a LRU-eviction cache of fds in this pollable + cached_fd fd_cache[MAX_PROBE_EPOLL_FDS]; + int fd_cache_size; + uint64_t fd_cache_counter; }; static const char* pollable_type_string(pollable_type t) { @@ -145,8 +157,11 @@ static void pollable_unref(pollable* p, int line, const char* reason); * Fd Declarations */ +static gpr_atm g_fd_salt; + struct grpc_fd { int fd; + intptr_t salt; /* refst format: bit 0 : 1=Active / 0=Orphaned bits 1-n : refcount @@ -354,6 +369,7 @@ static grpc_fd* fd_create(int fd, const char* name) { new_fd->pollable_obj = nullptr; gpr_atm_rel_store(&new_fd->refst, (gpr_atm)1); new_fd->fd = fd; + new_fd->salt = gpr_atm_no_barrier_fetch_add(&g_fd_salt, 1); new_fd->read_closure->InitEvent(); new_fd->write_closure->InitEvent(); gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL); @@ -484,6 +500,8 @@ static grpc_error* pollable_create(pollable_type type, pollable** p) { (*p)->root_worker = nullptr; (*p)->event_cursor = 0; (*p)->event_count = 0; + (*p)->fd_cache_size = 0; + (*p)->fd_cache_counter = 0; return GRPC_ERROR_NONE; } @@ -524,7 +542,36 @@ static grpc_error* pollable_add_fd(pollable* p, grpc_fd* fd) { grpc_error* error = GRPC_ERROR_NONE; static const char* err_desc = "pollable_add_fd"; const int epfd = p->epfd; + gpr_mu_lock(&p->mu); + p->fd_cache_counter++; + // Handle the case of overflow for our cache counter by + // reseting the recency-counter on all cache objects + if (p->fd_cache_counter == 0) { + for (int i = 0; i < p->fd_cache_size; i++) { + p->fd_cache[i].last_used = 0; + } + } + int lru_idx = 0; + for (int i = 0; i < p->fd_cache_size; i++) { + if (p->fd_cache[i].fd == fd->fd && p->fd_cache[i].salt == fd->salt) { + GRPC_STATS_INC_POLLSET_FD_CACHE_HITS(); + p->fd_cache[i].last_used = p->fd_cache_counter; + gpr_mu_unlock(&p->mu); + return GRPC_ERROR_NONE; + } else if (p->fd_cache[i].last_used < p->fd_cache[lru_idx].last_used) { + lru_idx = i; + } + } + // Add to cache + if (p->fd_cache_size < MAX_PROBE_EPOLL_FDS) { + lru_idx = p->fd_cache_size; + p->fd_cache_size++; + } + p->fd_cache[lru_idx].fd = fd->fd; + p->fd_cache[lru_idx].salt = fd->salt; + p->fd_cache[lru_idx].last_used = p->fd_cache_counter; + gpr_mu_unlock(&p->mu); if (grpc_polling_trace.enabled()) { gpr_log(GPR_INFO, "add fd %p (%d) to pollable %p", fd, fd->fd, p); } @@ -533,6 +580,7 @@ static grpc_error* pollable_add_fd(pollable* p, grpc_fd* fd) { ev_fd.events = static_cast<uint32_t>(EPOLLET | EPOLLIN | EPOLLOUT | EPOLLEXCLUSIVE); ev_fd.data.ptr = fd; + GRPC_STATS_INC_SYSCALL_EPOLL_CTL(); if (epoll_ctl(epfd, EPOLL_CTL_ADD, fd->fd, &ev_fd) != 0) { switch (errno) { case EEXIST: diff --git a/src/core/lib/iomgr/resource_quota.cc b/src/core/lib/iomgr/resource_quota.cc index 8cf4fe9928..539bc120ce 100644 --- a/src/core/lib/iomgr/resource_quota.cc +++ b/src/core/lib/iomgr/resource_quota.cc @@ -386,7 +386,7 @@ static bool rq_reclaim(grpc_resource_quota* resource_quota, bool destructive) { resource_quota->debug_only_last_reclaimer_resource_user = resource_user; resource_quota->debug_only_last_initiated_reclaimer = c; resource_user->reclaimers[destructive] = nullptr; - GRPC_CLOSURE_RUN(c, GRPC_ERROR_NONE); + GRPC_CLOSURE_SCHED(c, GRPC_ERROR_NONE); return true; } diff --git a/src/core/lib/iomgr/tcp_client_posix.cc b/src/core/lib/iomgr/tcp_client_posix.cc index 6144d389f7..900c056575 100644 --- a/src/core/lib/iomgr/tcp_client_posix.cc +++ b/src/core/lib/iomgr/tcp_client_posix.cc @@ -45,6 +45,7 @@ #include "src/core/lib/iomgr/tcp_posix.h" #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/unix_sockets_posix.h" +#include "src/core/lib/slice/slice_internal.h" extern grpc_core::TraceFlag grpc_tcp_trace; @@ -233,7 +234,7 @@ finish: error = grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS, addr_str_slice /* takes ownership */); } else { - grpc_slice_unref(addr_str_slice); + grpc_slice_unref_internal(addr_str_slice); } if (done) { // This is safe even outside the lock, because "done", the sentinel, is diff --git a/src/core/lib/iomgr/tcp_custom.cc b/src/core/lib/iomgr/tcp_custom.cc index b3b2934014..990e8d632b 100644 --- a/src/core/lib/iomgr/tcp_custom.cc +++ b/src/core/lib/iomgr/tcp_custom.cc @@ -141,7 +141,7 @@ static void call_read_cb(custom_tcp_endpoint* tcp, grpc_error* error) { TCP_UNREF(tcp, "read"); tcp->read_slices = nullptr; tcp->read_cb = nullptr; - GRPC_CLOSURE_RUN(cb, error); + GRPC_CLOSURE_SCHED(cb, error); } static void custom_read_callback(grpc_custom_socket* socket, size_t nread, diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index 153be05e83..b79ffe20f1 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -366,7 +366,7 @@ static void call_read_cb(grpc_tcp* tcp, grpc_error* error) { tcp->read_cb = nullptr; tcp->incoming_buffer = nullptr; - GRPC_CLOSURE_RUN(cb, error); + GRPC_CLOSURE_SCHED(cb, error); } #define MAX_READ_IOVEC 4 @@ -629,7 +629,7 @@ static void tcp_handle_write(void* arg /* grpc_tcp */, grpc_error* error) { gpr_log(GPR_INFO, "write: %s", str); } - GRPC_CLOSURE_RUN(cb, error); + GRPC_CLOSURE_SCHED(cb, error); TCP_UNREF(tcp, "write"); } } diff --git a/src/core/lib/iomgr/tcp_server_posix.cc b/src/core/lib/iomgr/tcp_server_posix.cc index 153ac63424..484d2b6077 100644 --- a/src/core/lib/iomgr/tcp_server_posix.cc +++ b/src/core/lib/iomgr/tcp_server_posix.cc @@ -187,11 +187,6 @@ static void on_read(void* arg, grpc_error* err) { goto error; } - read_notifier_pollset = - sp->server->pollsets[static_cast<size_t>(gpr_atm_no_barrier_fetch_add( - &sp->server->next_pollset_to_assign, 1)) % - sp->server->pollset_count]; - /* loop until accept4 returns EAGAIN, and then re-arm notification */ for (;;) { grpc_resolved_address addr; @@ -233,6 +228,11 @@ static void on_read(void* arg, grpc_error* err) { grpc_fd* fdobj = grpc_fd_create(fd, name); + read_notifier_pollset = + sp->server->pollsets[static_cast<size_t>(gpr_atm_no_barrier_fetch_add( + &sp->server->next_pollset_to_assign, 1)) % + sp->server->pollset_count]; + grpc_pollset_add_fd(read_notifier_pollset, fdobj); // Create acceptor. diff --git a/src/core/lib/security/security_connector/alts_security_connector.cc b/src/core/lib/security/security_connector/alts_security_connector.cc index 5ff7d7938b..35a787871a 100644 --- a/src/core/lib/security/security_connector/alts_security_connector.cc +++ b/src/core/lib/security/security_connector/alts_security_connector.cc @@ -30,6 +30,7 @@ #include "src/core/lib/security/credentials/alts/alts_credentials.h" #include "src/core/lib/security/transport/security_handshaker.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/transport/transport.h" #include "src/core/tsi/alts/handshaker/alts_tsi_handshaker.h" @@ -133,7 +134,7 @@ grpc_security_status grpc_alts_auth_context_from_tsi_peer( rpc_versions_prop->value.data, rpc_versions_prop->value.length); bool decode_result = grpc_gcp_rpc_protocol_versions_decode(slice, &peer_versions); - grpc_slice_unref(slice); + grpc_slice_unref_internal(slice); if (!decode_result) { gpr_log(GPR_ERROR, "Invalid peer rpc protocol versions."); return GRPC_SECURITY_ERROR; diff --git a/src/core/lib/security/security_connector/security_connector.cc b/src/core/lib/security/security_connector/security_connector.cc index a30696703f..b54a7643e4 100644 --- a/src/core/lib/security/security_connector/security_connector.cc +++ b/src/core/lib/security/security_connector/security_connector.cc @@ -69,8 +69,11 @@ void grpc_set_ssl_roots_override_callback(grpc_ssl_roots_override_callback cb) { /* Defines the cipher suites that we accept by default. All these cipher suites are compliant with HTTP2. */ -#define GRPC_SSL_CIPHER_SUITES \ - "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384" +#define GRPC_SSL_CIPHER_SUITES \ + "ECDHE-ECDSA-AES128-GCM-SHA256:" \ + "ECDHE-ECDSA-AES256-GCM-SHA384:" \ + "ECDHE-RSA-AES128-GCM-SHA256:" \ + "ECDHE-RSA-AES256-GCM-SHA384" static gpr_once cipher_suites_once = GPR_ONCE_INIT; static const char* cipher_suites = nullptr; diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index da488034ca..7ed1696f80 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -1259,8 +1259,12 @@ static void post_batch_completion(batch_control* bctl) { if (bctl->completion_data.notify_tag.is_closure) { /* unrefs bctl->error */ bctl->call = nullptr; - GRPC_CLOSURE_RUN((grpc_closure*)bctl->completion_data.notify_tag.tag, - error); + /* This closure may be meant to be run within some combiner. Since we aren't + * running in any combiner here, we need to use GRPC_CLOSURE_SCHED instead + * of GRPC_CLOSURE_RUN. + */ + GRPC_CLOSURE_SCHED((grpc_closure*)bctl->completion_data.notify_tag.tag, + error); GRPC_CALL_INTERNAL_UNREF(call, "completion"); } else { /* unrefs bctl->error */ diff --git a/src/core/lib/transport/byte_stream.cc b/src/core/lib/transport/byte_stream.cc index cb15a71a91..16b85ca0db 100644 --- a/src/core/lib/transport/byte_stream.cc +++ b/src/core/lib/transport/byte_stream.cc @@ -45,7 +45,7 @@ SliceBufferByteStream::SliceBufferByteStream(grpc_slice_buffer* slice_buffer, SliceBufferByteStream::~SliceBufferByteStream() {} void SliceBufferByteStream::Orphan() { - grpc_slice_buffer_destroy(&backing_buffer_); + grpc_slice_buffer_destroy_internal(&backing_buffer_); GRPC_ERROR_UNREF(shutdown_error_); // Note: We do not actually delete the object here, since // SliceBufferByteStream is usually allocated as part of a larger diff --git a/src/objective-c/GRPCClient/private/GRPCChannel.m b/src/objective-c/GRPCClient/private/GRPCChannel.m index a49a489ea8..b1f6ea270e 100644 --- a/src/objective-c/GRPCClient/private/GRPCChannel.m +++ b/src/objective-c/GRPCClient/private/GRPCChannel.m @@ -56,6 +56,7 @@ static void FreeChannelArgs(grpc_channel_args *channel_args) { gpr_free(arg->value.string); } } + gpr_free(channel_args->args); gpr_free(channel_args); } diff --git a/src/objective-c/GRPCClient/private/GRPCHost.m b/src/objective-c/GRPCClient/private/GRPCHost.m index bfb1fd352c..c3ea9afc37 100644 --- a/src/objective-c/GRPCClient/private/GRPCHost.m +++ b/src/objective-c/GRPCClient/private/GRPCHost.m @@ -50,6 +50,7 @@ static NSMutableDictionary *kHostCache; if (_channelCreds != nil) { grpc_channel_credentials_release(_channelCreds); } + [GRPCConnectivityMonitor unregisterObserver:self]; } // Default initializer. @@ -278,7 +279,7 @@ static NSMutableDictionary *kHostCache; // and Cellular data, so that a new call will use a new channel. Otherwise, a new call will still // use the cached channel which is no longer available and will cause gRPC to hang. - (void)connectivityChange:(NSNotification *)note { - [GRPCHost flushChannelCache]; + [self disconnect]; } @end diff --git a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m index af1ce0bf23..730a1436e4 100644 --- a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m +++ b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m @@ -54,7 +54,7 @@ + (instancetype)grpc_stringFromMetadataValue:(grpc_metadata *)metadata { return [[self alloc] initWithBytes:GRPC_SLICE_START_PTR(metadata->value) length:GRPC_SLICE_LENGTH(metadata->value) - encoding:NSASCIIStringEncoding]; + encoding:NSUTF8StringEncoding]; } // Precondition: This object contains only ASCII characters. diff --git a/src/objective-c/GRPCClient/private/NSError+GRPC.m b/src/objective-c/GRPCClient/private/NSError+GRPC.m index 74cfa943cc..c2e65e4d8a 100644 --- a/src/objective-c/GRPCClient/private/NSError+GRPC.m +++ b/src/objective-c/GRPCClient/private/NSError+GRPC.m @@ -27,7 +27,7 @@ NSString *const kGRPCErrorDomain = @"io.grpc"; if (statusCode == GRPC_STATUS_OK) { return nil; } - NSString *message = [NSString stringWithCString:details encoding:NSASCIIStringEncoding]; + NSString *message = [NSString stringWithCString:details encoding:NSUTF8StringEncoding]; return [NSError errorWithDomain:kGRPCErrorDomain code:statusCode userInfo:@{NSLocalizedDescriptionKey : message}]; diff --git a/src/php/lib/Grpc/BaseStub.php b/src/php/lib/Grpc/BaseStub.php index 7860233ca2..b9c50b1da6 100644 --- a/src/php/lib/Grpc/BaseStub.php +++ b/src/php/lib/Grpc/BaseStub.php @@ -60,7 +60,7 @@ class BaseStub } if ($channel) { if (!is_a($channel, 'Grpc\Channel') && - !is_a($channel, 'Grpc\InterceptorChannel')) { + !is_a($channel, 'Grpc\Internal\InterceptorChannel')) { throw new \Exception('The channel argument is not a Channel object '. 'or an InterceptorChannel object created by '. 'Interceptor::intercept($channel, Interceptor|Interceptor[] $interceptors)'); @@ -69,6 +69,18 @@ class BaseStub return; } + $this->channel = static::getDefaultChannel($hostname, $opts); + } + + /** + * Creates and returns the default Channel + * + * @param array $opts Channel constructor options + * + * @return Channel The channel + */ + public static function getDefaultChannel($hostname, array $opts) + { $package_config = json_decode( file_get_contents(dirname(__FILE__).'/../../composer.json'), true @@ -85,7 +97,7 @@ class BaseStub 'required. Please see one of the '. 'ChannelCredentials::create methods'); } - $this->channel = new Channel($hostname, $opts); + return new Channel($hostname, $opts); } /** @@ -365,7 +377,7 @@ class BaseStub */ private function _UnaryUnaryCallFactory($channel, $deserialize) { - if (is_a($channel, 'Grpc\InterceptorChannel')) { + if (is_a($channel, 'Grpc\Internal\InterceptorChannel')) { return function ($method, $argument, array $metadata = [], @@ -392,7 +404,7 @@ class BaseStub */ private function _UnaryStreamCallFactory($channel, $deserialize) { - if (is_a($channel, 'Grpc\InterceptorChannel')) { + if (is_a($channel, 'Grpc\Internal\InterceptorChannel')) { return function ($method, $argument, array $metadata = [], @@ -419,7 +431,7 @@ class BaseStub */ private function _StreamUnaryCallFactory($channel, $deserialize) { - if (is_a($channel, 'Grpc\InterceptorChannel')) { + if (is_a($channel, 'Grpc\Internal\InterceptorChannel')) { return function ($method, array $metadata = [], array $options = []) use ($channel, $deserialize) { @@ -444,7 +456,7 @@ class BaseStub */ private function _StreamStreamCallFactory($channel, $deserialize) { - if (is_a($channel, 'Grpc\InterceptorChannel')) { + if (is_a($channel, 'Grpc\Internal\InterceptorChannel')) { return function ($method, array $metadata = [], array $options = []) use ($channel, $deserialize) { diff --git a/src/php/lib/Grpc/Interceptor.php b/src/php/lib/Grpc/Interceptor.php index 9c1b5616f2..e1b97f2a84 100644 --- a/src/php/lib/Grpc/Interceptor.php +++ b/src/php/lib/Grpc/Interceptor.php @@ -75,10 +75,10 @@ class Interceptor { if (is_array($interceptors)) { for ($i = count($interceptors) - 1; $i >= 0; $i--) { - $channel = new InterceptorChannel($channel, $interceptors[$i]); + $channel = new Internal\InterceptorChannel($channel, $interceptors[$i]); } } else { - $channel = new InterceptorChannel($channel, $interceptors); + $channel = new Internal\InterceptorChannel($channel, $interceptors); } return $channel; } diff --git a/src/php/lib/Grpc/Internal/InterceptorChannel.php b/src/php/lib/Grpc/Internal/InterceptorChannel.php index 9ac05748f3..2f85c35fe0 100644 --- a/src/php/lib/Grpc/Internal/InterceptorChannel.php +++ b/src/php/lib/Grpc/Internal/InterceptorChannel.php @@ -17,12 +17,12 @@ * */ -namespace Grpc; +namespace Grpc\Internal; /** * This is a PRIVATE API and can change without notice. */ -class InterceptorChannel +class InterceptorChannel extends \Grpc\Channel { private $next = null; private $interceptor; @@ -35,7 +35,7 @@ class InterceptorChannel public function __construct($channel, $interceptor) { if (!is_a($channel, 'Grpc\Channel') && - !is_a($channel, 'Grpc\InterceptorChannel')) { + !is_a($channel, 'Grpc\Internal\InterceptorChannel')) { throw new \Exception('The channel argument is not a Channel object '. 'or an InterceptorChannel object created by '. 'Interceptor::intercept($channel, Interceptor|Interceptor[] $interceptors)'); diff --git a/src/php/tests/unit_tests/InterceptorTest.php b/src/php/tests/unit_tests/InterceptorTest.php index 08f5abbb21..11c5b4325a 100644 --- a/src/php/tests/unit_tests/InterceptorTest.php +++ b/src/php/tests/unit_tests/InterceptorTest.php @@ -58,7 +58,7 @@ class InterceptorClient extends Grpc\BaseStub /** * A simple RPC. - * @param \Routeguide\Point $argument input argument + * @param SimpleRequest $argument input argument * @param array $metadata metadata * @param array $options call options */ @@ -221,15 +221,11 @@ class InterceptorTest extends PHPUnit_Framework_TestCase $req_text = 'client_request'; $channel_matadata_interceptor = new ChangeMetadataInterceptor(); $intercept_channel = Grpc\Interceptor::intercept($this->channel, $channel_matadata_interceptor); - echo "create Client\n"; $client = new InterceptorClient('localhost:'.$this->port, [ 'credentials' => Grpc\ChannelCredentials::createInsecure(), ], $intercept_channel); - echo "create Call\n"; $req = new SimpleRequest($req_text); - echo "Call created\n"; $unary_call = $client->UnaryCall($req); - echo "start call\n"; $event = $this->server->requestCall(); $this->assertSame('/dummy_method', $event->method); $this->assertSame(['interceptor_from_unary_request'], $event->metadata['foo']); diff --git a/src/proto/grpc/lb/v1/load_balancer.proto b/src/proto/grpc/lb/v1/load_balancer.proto index 75c916defa..3e0a4b2c97 100644 --- a/src/proto/grpc/lb/v1/load_balancer.proto +++ b/src/proto/grpc/lb/v1/load_balancer.proto @@ -62,8 +62,10 @@ message LoadBalanceRequest { } message InitialLoadBalanceRequest { - // Name of load balanced service (IE, balancer.service.com) - // length should be less than 256 bytes. + // The name of the load balanced service (e.g., balancer.service.com). The max + // length of the name is 256 bytes. + // The name might include a port number. How to handle the port number is up + // to the balancer. string name = 1; } diff --git a/src/python/grpcio/grpc/_channel.py b/src/python/grpcio/grpc/_channel.py index 8cc0e981ef..2017d47130 100644 --- a/src/python/grpcio/grpc/_channel.py +++ b/src/python/grpcio/grpc/_channel.py @@ -58,6 +58,17 @@ _STREAM_STREAM_INITIAL_DUE = ( _CHANNEL_SUBSCRIPTION_CALLBACK_ERROR_LOG_MESSAGE = ( 'Exception calling channel subscription callback!') +_OK_RENDEZVOUS_REPR_FORMAT = ('<_Rendezvous of RPC that terminated with:\n' + '\tstatus = {}\n' + '\tdetails = "{}"\n' + '>') + +_NON_OK_RENDEZVOUS_REPR_FORMAT = ('<_Rendezvous of RPC that terminated with:\n' + '\tstatus = {}\n' + '\tdetails = "{}"\n' + '\tdebug_error_string = "{}"\n' + '>') + def _deadline(timeout): return None if timeout is None else time.time() + timeout @@ -91,6 +102,7 @@ class _RPCState(object): self.trailing_metadata = trailing_metadata self.code = code self.details = details + self.debug_error_string = None # The semantics of grpc.Future.cancel and grpc.Future.cancelled are # slightly wonky, so they have to be tracked separately from the rest of the # result of the RPC. This field tracks whether cancellation was requested @@ -137,6 +149,7 @@ def _handle_event(event, state, response_deserializer): else: state.code = code state.details = batch_operation.details() + state.debug_error_string = batch_operation.error_string() callbacks.extend(state.callbacks) state.callbacks = None return callbacks @@ -374,13 +387,23 @@ class _Rendezvous(grpc.RpcError, grpc.Future, grpc.Call): self._state.condition.wait() return _common.decode(self._state.details) + def debug_error_string(self): + with self._state.condition: + while self._state.debug_error_string is None: + self._state.condition.wait() + return _common.decode(self._state.debug_error_string) + def _repr(self): with self._state.condition: if self._state.code is None: return '<_Rendezvous object of in-flight RPC>' + elif self._state.code is grpc.StatusCode.OK: + return _OK_RENDEZVOUS_REPR_FORMAT.format( + self._state.code, self._state.details) else: - return '<_Rendezvous of RPC that terminated with ({}, {})>'.format( - self._state.code, _common.decode(self._state.details)) + return _NON_OK_RENDEZVOUS_REPR_FORMAT.format( + self._state.code, self._state.details, + self._state.debug_error_string) def __repr__(self): return self._repr() diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi index a4c0319553..2d6c900c54 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi @@ -291,6 +291,7 @@ cdef extern from "grpc/grpc.h": grpc_metadata_array *trailing_metadata grpc_status_code *status grpc_slice *status_details + char** error_string ctypedef struct grpc_op_data_recv_close_on_server: int *cancelled diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/operation.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/operation.pxd.pxi index bfbe27785b..69a2a4989e 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/operation.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/operation.pxd.pxi @@ -91,9 +91,11 @@ cdef class ReceiveStatusOnClientOperation(Operation): cdef grpc_metadata_array _c_trailing_metadata cdef grpc_status_code _c_code cdef grpc_slice _c_details + cdef const char* _c_error_string cdef tuple _trailing_metadata cdef object _code cdef str _details + cdef str _error_string cdef void c(self) cdef void un_c(self) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/operation.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/operation.pyx.pxi index 239d0f3f95..454627f570 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/operation.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/operation.pyx.pxi @@ -199,6 +199,8 @@ cdef class ReceiveStatusOnClientOperation(Operation): &self._c_code) self.c_op.data.receive_status_on_client.status_details = ( &self._c_details) + self.c_op.data.receive_status_on_client.error_string = ( + &self._c_error_string) cdef void un_c(self): self._trailing_metadata = _metadata(&self._c_trailing_metadata) @@ -206,6 +208,11 @@ cdef class ReceiveStatusOnClientOperation(Operation): self._code = self._c_code self._details = _decode(_slice_bytes(self._c_details)) grpc_slice_unref(self._c_details) + if self._c_error_string != NULL: + self._error_string = _decode(self._c_error_string) + gpr_free(<void*>self._c_error_string) + else: + self._error_string = "" def trailing_metadata(self): return self._trailing_metadata @@ -216,6 +223,9 @@ cdef class ReceiveStatusOnClientOperation(Operation): def details(self): return self._details + def error_string(self): + return self._error_string + cdef class ReceiveCloseOnServerOperation(Operation): diff --git a/src/python/grpcio_tests/tests/unit/_rpc_test.py b/src/python/grpcio_tests/tests/unit/_rpc_test.py index 54f01d9f8d..34e7831a98 100644 --- a/src/python/grpcio_tests/tests/unit/_rpc_test.py +++ b/src/python/grpcio_tests/tests/unit/_rpc_test.py @@ -225,6 +225,7 @@ class RPCTest(unittest.TestCase): self.assertEqual(expected_response, response) self.assertIs(grpc.StatusCode.OK, call.code()) + self.assertEqual("", call.debug_error_string()) def testSuccessfulUnaryRequestFutureUnaryResponse(self): request = b'\x07\x08' @@ -706,6 +707,13 @@ class RPCTest(unittest.TestCase): self.assertIs(grpc.StatusCode.UNKNOWN, exception_context.exception.code()) + # sanity checks on to make sure returned string contains default members + # of the error + debug_error_string = exception_context.exception.debug_error_string() + self.assertIn("created", debug_error_string) + self.assertIn("description", debug_error_string) + self.assertIn("file", debug_error_string) + self.assertIn("file_line", debug_error_string) def testFailedUnaryRequestFutureUnaryResponse(self): request = b'\x37\x17' |