Merge branch 'metadata_filter' of github.com:ctiller/grpc into metadata_filter

.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/objective-c/GRPCClient/private/GRPCChannel.m

@@ -200,7 +200,7 @@ static grpc_channel_args *BuildChannelArgs(NSDictionary *dictionary) {
   return grpc_channel_create_call(_unmanagedChannel,
                                   NULL, GRPC_PROPAGATE_DEFAULTS,
                                   queue.unmanagedQueue,
-                                  path.UTF8String,
+                                  grpc_slice_from_copied_string(path.UTF8String),
                                   NULL, // Passing NULL for host
                                   gpr_inf_future(GPR_CLOCK_REALTIME), NULL);
 }

src/objective-c/GRPCClient/private/GRPCWrappedCall.m

@@ -194,7 +194,7 @@
 
 @implementation GRPCOpRecvStatus{
   grpc_status_code _statusCode;
-  char *_details;
+  grpc_slice _details;
   size_t _detailsCapacity;
   grpc_metadata_array _trailers;
 }
@@ -208,7 +208,6 @@
     _op.op = GRPC_OP_RECV_STATUS_ON_CLIENT;
     _op.data.recv_status_on_client.status = &_statusCode;
     _op.data.recv_status_on_client.status_details = &_details;
-    _op.data.recv_status_on_client.status_details_capacity = &_detailsCapacity;
     grpc_metadata_array_init(&_trailers);
     _op.data.recv_status_on_client.trailing_metadata = &_trailers;
     if (handler) {
@@ -216,11 +215,15 @@
       __weak typeof(self) weakSelf = self;
       _handler = ^{
         __strong typeof(self) strongSelf = weakSelf;
-        NSError *error = [NSError grpc_errorFromStatusCode:strongSelf->_statusCode
-                                                   details:strongSelf->_details];
-        NSDictionary *trailers = [NSDictionary
-                                  grpc_dictionaryFromMetadataArray:strongSelf->_trailers];
-        handler(error, trailers);
+        if (strongSelf) {
+          char *details = grpc_slice_to_c_string(strongSelf->_details);
+          NSError *error = [NSError grpc_errorFromStatusCode:strongSelf->_statusCode
+                                                     details:details];
+          NSDictionary *trailers = [NSDictionary
+                                    grpc_dictionaryFromMetadataArray:strongSelf->_trailers];
+          handler(error, trailers);
+          gpr_free(details);
+        }
       };
     }
   }
@@ -229,7 +232,7 @@
 
 - (void)dealloc {
   grpc_metadata_array_destroy(&_trailers);
-  gpr_free(_details);
+  grpc_slice_unref(_details);
 }
 
 @end

src/objective-c/GRPCClient/private/NSDictionary+GRPC.m

@@ -47,12 +47,12 @@
 @implementation NSData (GRPCMetadata)
 + (instancetype)grpc_dataFromMetadataValue:(grpc_metadata *)metadata {
   // TODO(jcanizales): Should we use a non-copy constructor?
-  return [self dataWithBytes:metadata->value length:metadata->value_length];
+  return [self dataWithBytes:GRPC_SLICE_START_PTR(metadata->value)
+                      length:GRPC_SLICE_LENGTH(metadata->value)];
 }
 
 - (void)grpc_initMetadata:(grpc_metadata *)metadata {
-  metadata->value = self.bytes;
-  metadata->value_length = self.length;
+  metadata->value = grpc_slice_from_copied_buffer(self.bytes, self.length);
 }
 @end
 
@@ -67,15 +67,14 @@
 
 @implementation NSString (GRPCMetadata)
 + (instancetype)grpc_stringFromMetadataValue:(grpc_metadata *)metadata {
-  return [[self alloc] initWithBytes:metadata->value
-                              length:metadata->value_length
+  return [[self alloc] initWithBytes:GRPC_SLICE_START_PTR(metadata->value)
+                              length:GRPC_SLICE_LENGTH(metadata->value)
                             encoding:NSASCIIStringEncoding];
 }
 
 // Precondition: This object contains only ASCII characters.
 - (void)grpc_initMetadata:(grpc_metadata *)metadata {
-  metadata->value = self.UTF8String;
-  metadata->value_length = self.length;
+  metadata->value = grpc_slice_from_copied_string(self.UTF8String);
 }
 @end
 
@@ -89,7 +88,10 @@
 + (instancetype)grpc_dictionaryFromMetadata:(grpc_metadata *)entries count:(size_t)count {
   NSMutableDictionary *metadata = [NSMutableDictionary dictionaryWithCapacity:count];
   for (grpc_metadata *entry = entries; entry < entries + count; entry++) {
-    NSString *name = [NSString stringWithCString:entry->key encoding:NSASCIIStringEncoding];
+    char *key = grpc_slice_to_c_string(entry->key);
+    NSString *name = [NSString stringWithCString:key
+                                        encoding:NSASCIIStringEncoding];
+    gpr_free(key);
     if (!name || metadata[name]) {
       // Log if name is nil?
       continue;
@@ -112,7 +114,7 @@
   grpc_metadata *current = metadata;
   for (NSString* key in self) {
     id value = self[key];
-    current->key = key.UTF8String;
+    current->key = grpc_slice_from_copied_string(key.UTF8String);
     if ([value respondsToSelector:@selector(grpc_initMetadata:)]) {
       [value grpc_initMetadata:current];
     } else {

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: