Paper-over Python errors

This reads directly off of the slices rather than ref'ing and unref'ing
them. There's likely some silliness w.r.t. interned slices and
references to them outliving their associated call objects, but we are
just ignoring that for now.

.gitignore

@@ -31,7 +31,7 @@ coverage
 # python compiled objects
 *.pyc
 
-#eclipse project files
+# eclipse project files
 .cproject
 .project
 .settings
@@ -110,3 +110,6 @@ bazel-genfiles
 bazel-grpc
 bazel-out
 bazel-testlogs
+
+# Debug output
+gdb.txt

src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi

@@ -70,6 +70,8 @@ cdef class CompletionQueue:
  operation_call = tag.operation_call
  request_call_details = tag.request_call_details
  request_metadata = tag.request_metadata
+ if request_metadata is not None:
+ request_metadata._claim_slice_ownership()
  batch_operations = tag.batch_operations
  if tag.is_new_request:
  # Stuff in the tag not explicitly handled by us needs to live through
@@ -91,7 +93,7 @@ cdef class CompletionQueue:
  c_deadline = gpr_inf_future(GPR_CLOCK_REALTIME)
  if deadline is not None:
  c_deadline = deadline.c_time
- 
+
  while True:
  c_timeout = gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), c_increment)
  if gpr_time_cmp(c_timeout, c_deadline) > 0:
@@ -100,7 +102,7 @@ cdef class CompletionQueue:
  self.c_completion_queue, c_timeout, NULL)
  if event.type != GRPC_QUEUE_TIMEOUT or gpr_time_cmp(c_timeout, c_deadline) == 0:
  break;
- 
+
  # Handle any signals
  with gil:
  cpython.PyErr_CheckSignals()

src/python/grpcio/grpc/_cython/_cygrpc/records.pxd.pxi

@@ -77,6 +77,8 @@ cdef class Slice:
  cdef void _assign_slice(self, grpc_slice new_slice) nogil
  @staticmethod
  cdef Slice from_slice(grpc_slice slice)
+ @staticmethod
+ cdef bytes bytes_from_slice(grpc_slice slice)
 
 
 cdef class ByteBuffer:
@@ -113,7 +115,10 @@ cdef class Metadatum:
 cdef class Metadata:
 
  cdef grpc_metadata_array c_metadata_array
+ cdef bint owns_metadata_slices
  cdef object metadata
+ cdef void _claim_slice_ownership(self) nogil
+ cdef void _drop_slice_ownership(self) nogil
 
 
 cdef class Operation:

src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi

@@ -189,11 +189,11 @@ cdef class CallDetails:
 
  @property
  def method(self):
- return Slice.from_slice(self.c_details.method).bytes()
+ return Slice.bytes_from_slice(self.c_details.method)
 
  @property
  def host(self):
- return Slice.from_slice(self.c_details.host).bytes()
+ return Slice.bytes_from_slice(self.c_details.host)
 
  @property
  def deadline(self):
@@ -251,12 +251,16 @@ cdef class Slice:
  self._assign_slice(slice)
  return self
 
- def bytes(self):
+ @staticmethod
+ cdef bytes bytes_from_slice(grpc_slice slice):
  with nogil:
- pointer = grpc_slice_start_ptr(self.c_slice)
- length = grpc_slice_length(self.c_slice)
+ pointer = grpc_slice_start_ptr(slice)
+ length = grpc_slice_length(slice)
  return (<char *>pointer)[:length]
 
+ def bytes(self):
+ return Slice.bytes_from_slice(self.c_slice)
+
  def __dealloc__(self):
  with nogil:
  grpc_slice_unref(self.c_slice)
@@ -466,13 +470,14 @@ cdef class _MetadataIterator:
 cdef class Metadata:
 
  def __cinit__(self, metadata):
- grpc_init()
+ with nogil:
+ grpc_init()
+ grpc_metadata_array_init(&self.c_metadata_array)
+ self.owns_metadata_slices = False
  self.metadata = list(metadata)
- for metadatum in metadata:
+ for metadatum in self.metadata:
  if not isinstance(metadatum, Metadatum):
  raise TypeError("expected list of Metadatum")
- with nogil:
- grpc_metadata_array_init(&self.c_metadata_array)
  self.c_metadata_array.count = len(self.metadata)
  self.c_metadata_array.capacity = len(self.metadata)
  with nogil:
@@ -484,23 +489,43 @@ cdef class Metadata:
  (<Metadatum>self.metadata[i]).c_metadata)
 
  def __dealloc__(self):
- # this frees the allocated memory for the grpc_metadata_array (although
- # it'd be nice if that were documented somewhere...)
- # TODO(atash): document this in the C core
- grpc_metadata_array_destroy(&self.c_metadata_array)
- grpc_shutdown()
+ with nogil:
+ self._drop_slice_ownership()
+ # this frees the allocated memory for the grpc_metadata_array (although
+ # it'd be nice if that were documented somewhere...)
+ # TODO(atash): document this in the C core
+ grpc_metadata_array_destroy(&self.c_metadata_array)
+ grpc_shutdown()
 
  def __len__(self):
  return self.c_metadata_array.count
 
  def __getitem__(self, size_t i):
+ if i >= self.c_metadata_array.count:
+ raise IndexError
  return Metadatum(
- key=Slice.from_slice(self.c_metadata_array.metadata[i].key).bytes(),
- value=Slice.from_slice(self.c_metadata_array.metadata[i].value).bytes())
+ key=Slice.bytes_from_slice(self.c_metadata_array.metadata[i].key),
+ value=Slice.bytes_from_slice(self.c_metadata_array.metadata[i].value))
 
  def __iter__(self):
  return _MetadataIterator(self)
 
+ cdef void _claim_slice_ownership(self) nogil:
+ if self.owns_metadata_slices:
+ return
+ for i in range(self.c_metadata_array.count):
+ grpc_slice_ref(self.c_metadata_array.metadata[i].key)
+ grpc_slice_ref(self.c_metadata_array.metadata[i].value)
+ self.owns_metadata_slices = True
+
+ cdef void _drop_slice_ownership(self) nogil:
+ if not self.owns_metadata_slices:
+ return
+ for i in range(self.c_metadata_array.count):
+ grpc_slice_unref(self.c_metadata_array.metadata[i].key)
+ grpc_slice_unref(self.c_metadata_array.metadata[i].value)
+ self.owns_metadata_slices = False
+
 
 cdef class Operation:
 

src/python/grpcio/grpc/_server.py

@@ -576,6 +576,8 @@ def _handle_with_method_handler(rpc_event, method_handler, thread_pool):
 
 
 def _handle_call(rpc_event, generic_handlers, thread_pool):
+ if not rpc_event.success:
+ return None
  if rpc_event.request_call_details.method is not None:
  method_handler = _find_method_handler(rpc_event, generic_handlers)
  if method_handler is None: