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

7 files changed, 114 insertions, 44 deletions

diff --git a/src/core/lib/iomgr/error.c b/src/core/lib/iomgr/error.c
index 7cdbe30198..1127fff756 100644
--- a/src/core/lib/iomgr/error.c
+++ b/src/core/lib/iomgr/error.c
@@ -140,14 +140,16 @@ grpc_error *grpc_error_ref(grpc_error *err, const char *file, int line,
                            const char *func) {
   if (grpc_error_is_special(err)) return err;
   gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
-          err->refs.count, err->refs.count + 1, file, line, func);
-  gpr_ref(&err->refs);
+          gpr_atm_no_barrier_load(&err->atomics.refs.count),
+          gpr_atm_no_barrier_load(&err->atomics.refs.count) + 1, file, line,
+          func);
+  gpr_ref(&err->atomics.refs);
   return err;
 }
 #else
 grpc_error *grpc_error_ref(grpc_error *err) {
   if (grpc_error_is_special(err)) return err;
-  gpr_ref(&err->refs);
+  gpr_ref(&err->atomics.refs);
   return err;
 }
 #endif
@@ -182,7 +184,7 @@ static void error_destroy(grpc_error *err) {
   GPR_ASSERT(!grpc_error_is_special(err));
   unref_errs(err);
   unref_strs(err);
-  gpr_free((void *)gpr_atm_acq_load(&err->error_string));
+  gpr_free((void *)gpr_atm_acq_load(&err->atomics.error_string));
   gpr_free(err);
 }
 
@@ -191,15 +193,17 @@ void grpc_error_unref(grpc_error *err, const char *file, int line,
                       const char *func) {
   if (grpc_error_is_special(err)) return;
   gpr_log(GPR_DEBUG, "%p: %" PRIdPTR " -> %" PRIdPTR " [%s:%d %s]", err,
-          err->refs.count, err->refs.count - 1, file, line, func);
-  if (gpr_unref(&err->refs)) {
+          gpr_atm_no_barrier_load(&err->atomics.refs.count),
+          gpr_atm_no_barrier_load(&err->atomics.refs.count) - 1, file, line,
+          func);
+  if (gpr_unref(&err->atomics.refs)) {
     error_destroy(err);
   }
 }
 #else
 void grpc_error_unref(grpc_error *err) {
   if (grpc_error_is_special(err)) return;
-  if (gpr_unref(&err->refs)) {
+  if (gpr_unref(&err->atomics.refs)) {
     error_destroy(err);
   }
 }
@@ -328,8 +332,8 @@ grpc_error *grpc_error_create(const char *file, int line, const char *desc,
 
   internal_set_time(&err, GRPC_ERROR_TIME_CREATED, gpr_now(GPR_CLOCK_REALTIME));
 
-  gpr_atm_no_barrier_store(&err->error_string, 0);
-  gpr_ref_init(&err->refs, 1);
+  gpr_atm_no_barrier_store(&err->atomics.error_string, 0);
+  gpr_ref_init(&err->atomics.refs, 1);
   GPR_TIMER_END("grpc_error_create", 0);
   return err;
 }
@@ -369,7 +373,7 @@ static grpc_error *copy_error_and_unref(grpc_error *in) {
                        grpc_slice_from_static_string("cancelled"));
       internal_set_int(&out, GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_CANCELLED);
     }
-  } else if (gpr_ref_is_unique(&in->refs)) {
+  } else if (gpr_ref_is_unique(&in->atomics.refs)) {
     out = in;
   } else {
     uint8_t new_arena_capacity = in->arena_capacity;
@@ -382,10 +386,14 @@ static grpc_error *copy_error_and_unref(grpc_error *in) {
 #ifdef GRPC_ERROR_REFCOUNT_DEBUG
     gpr_log(GPR_DEBUG, "%p create copying %p", out, in);
 #endif
-    memcpy(out, in, sizeof(*in) + in->arena_size * sizeof(intptr_t));
+    // bulk memcpy of the rest of the struct.
+    size_t skip = sizeof(&out->atomics);
+    memcpy((void *)((uintptr_t)out + skip), (void *)((uintptr_t)in + skip),
+           sizeof(*in) + (in->arena_size * sizeof(intptr_t)) - skip);
+    // manually set the atomics and the new capacity
+    gpr_atm_no_barrier_store(&out->atomics.error_string, 0);
+    gpr_ref_init(&out->atomics.refs, 1);
     out->arena_capacity = new_arena_capacity;
-    gpr_atm_no_barrier_store(&out->error_string, 0);
-    gpr_ref_init(&out->refs, 1);
     ref_strs(out);
     ref_errs(out);
     GRPC_ERROR_UNREF(in);
@@ -692,7 +700,7 @@ const char *grpc_error_string(grpc_error *err) {
   if (err == GRPC_ERROR_OOM) return oom_error_string;
   if (err == GRPC_ERROR_CANCELLED) return cancelled_error_string;
 
-  void *p = (void *)gpr_atm_acq_load(&err->error_string);
+  void *p = (void *)gpr_atm_acq_load(&err->atomics.error_string);
   if (p != NULL) {
     GPR_TIMER_END("grpc_error_string", 0);
     return p;
@@ -712,9 +720,9 @@ const char *grpc_error_string(grpc_error *err) {
 
   char *out = finish_kvs(&kvs);
 
-  if (!gpr_atm_rel_cas(&err->error_string, 0, (gpr_atm)out)) {
+  if (!gpr_atm_rel_cas(&err->atomics.error_string, 0, (gpr_atm)out)) {
     gpr_free(out);
-    out = (char *)gpr_atm_no_barrier_load(&err->error_string);
+    out = (char *)gpr_atm_no_barrier_load(&err->atomics.error_string);
   }
 
   GPR_TIMER_END("grpc_error_string", 0);
diff --git a/src/core/lib/iomgr/error_internal.h b/src/core/lib/iomgr/error_internal.h
index fb4814e41f..7f204df1b2 100644
--- a/src/core/lib/iomgr/error_internal.h
+++ b/src/core/lib/iomgr/error_internal.h
@@ -46,14 +46,25 @@ struct grpc_linked_error {
   uint8_t next;
 };
 
+// c core representation of an error. See error.h for high level description of
+// this object.
 struct grpc_error {
-  gpr_refcount refs;
+  // All atomics in grpc_error must be stored in this nested struct. The rest of
+  // the object is memcpy-ed in bulk in copy_and_unref.
+  struct atomics {
+    gpr_refcount refs;
+    gpr_atm error_string;
+  } atomics;
+  // These arrays index into dynamic arena at the bottom of the struct.
+  // UINT8_MAX is used as a sentinel value.
   uint8_t ints[GRPC_ERROR_INT_MAX];
   uint8_t strs[GRPC_ERROR_STR_MAX];
   uint8_t times[GRPC_ERROR_TIME_MAX];
+  // The child errors are stored in the arena, but are effectively a linked list
+  // structure, since they are contained withing grpc_linked_error objects.
   uint8_t first_err;
   uint8_t last_err;
-  gpr_atm error_string;
+  // The arena is dynamically reallocated with a grow factor of 1.5.
   uint8_t arena_size;
   uint8_t arena_capacity;
   intptr_t arena[0];
diff --git a/src/core/lib/iomgr/pollset_uv.c b/src/core/lib/iomgr/pollset_uv.c
index af33949c69..a2f81bcd78 100644
--- a/src/core/lib/iomgr/pollset_uv.c
+++ b/src/core/lib/iomgr/pollset_uv.c
@@ -39,6 +39,7 @@
 
 #include <string.h>
 
+#include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/sync.h>
 
@@ -61,25 +62,30 @@ gpr_mu grpc_polling_mu;
    immediately in the next loop iteration.
    Note: In the future, if there is a bug that involves missing wakeups in the
    future, try adding a uv_async_t to kick the loop differently */
-uv_timer_t dummy_uv_handle;
+uv_timer_t *dummy_uv_handle;
 
 size_t grpc_pollset_size() { return sizeof(grpc_pollset); }
 
 void dummy_timer_cb(uv_timer_t *handle) {}
 
+void dummy_handle_close_cb(uv_handle_t *handle) { gpr_free(handle); }
+
 void grpc_pollset_global_init(void) {
   gpr_mu_init(&grpc_polling_mu);
-  uv_timer_init(uv_default_loop(), &dummy_uv_handle);
+  dummy_uv_handle = gpr_malloc(sizeof(uv_timer_t));
+  uv_timer_init(uv_default_loop(), dummy_uv_handle);
   grpc_pollset_work_run_loop = 1;
 }
 
-static void timer_close_cb(uv_handle_t *handle) { handle->data = (void *)1; }
-
 void grpc_pollset_global_shutdown(void) {
   gpr_mu_destroy(&grpc_polling_mu);
-  uv_close((uv_handle_t *)&dummy_uv_handle, timer_close_cb);
+  uv_close((uv_handle_t *)dummy_uv_handle, dummy_handle_close_cb);
 }
 
+static void timer_run_cb(uv_timer_t *timer) {}
+
+static void timer_close_cb(uv_handle_t *handle) { handle->data = (void *)1; }
+
 void grpc_pollset_init(grpc_pollset *pollset, gpr_mu **mu) {
   *mu = &grpc_polling_mu;
   uv_timer_init(uv_default_loop(), &pollset->timer);
@@ -95,7 +101,7 @@ void grpc_pollset_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset,
     uv_run(uv_default_loop(), UV_RUN_NOWAIT);
   } else {
     // kick the loop once
-    uv_timer_start(&dummy_uv_handle, dummy_timer_cb, 0, 0);
+    uv_timer_start(dummy_uv_handle, dummy_timer_cb, 0, 0);
   }
   grpc_closure_sched(exec_ctx, closure, GRPC_ERROR_NONE);
 }
@@ -111,8 +117,6 @@ void grpc_pollset_destroy(grpc_pollset *pollset) {
   }
 }
 
-static void timer_run_cb(uv_timer_t *timer) {}
-
 grpc_error *grpc_pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset,
                               grpc_pollset_worker **worker_hdl,
                               gpr_timespec now, gpr_timespec deadline) {
@@ -145,7 +149,7 @@ grpc_error *grpc_pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset,
 
 grpc_error *grpc_pollset_kick(grpc_pollset *pollset,
                               grpc_pollset_worker *specific_worker) {
-  uv_timer_start(&dummy_uv_handle, dummy_timer_cb, 0, 0);
+  uv_timer_start(dummy_uv_handle, dummy_timer_cb, 0, 0);
   return GRPC_ERROR_NONE;
 }
 
diff --git a/src/core/lib/iomgr/resolve_address_uv.c b/src/core/lib/iomgr/resolve_address_uv.c
index 79ff910738..4d715be94c 100644
--- a/src/core/lib/iomgr/resolve_address_uv.c
+++ b/src/core/lib/iomgr/resolve_address_uv.c
@@ -40,6 +40,7 @@
 #include <grpc/support/host_port.h>
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
+#include <grpc/support/useful.h>
 
 #include "src/core/lib/iomgr/closure.h"
 #include "src/core/lib/iomgr/error.h"
@@ -54,8 +55,36 @@ typedef struct request {
   grpc_closure *on_done;
   grpc_resolved_addresses **addresses;
   struct addrinfo *hints;
+  char *host;
+  char *port;
 } request;
 
+static int retry_named_port_failure(int status, request *r,
+                                    uv_getaddrinfo_cb getaddrinfo_cb) {
+  if (status != 0) {
+    // This loop is copied from resolve_address_posix.c
+    char *svc[][2] = {{"http", "80"}, {"https", "443"}};
+    for (size_t i = 0; i < GPR_ARRAY_SIZE(svc); i++) {
+      if (strcmp(r->port, svc[i][0]) == 0) {
+        int retry_status;
+        uv_getaddrinfo_t *req = gpr_malloc(sizeof(uv_getaddrinfo_t));
+        req->data = r;
+        retry_status = uv_getaddrinfo(uv_default_loop(), req, getaddrinfo_cb,
+                                      r->host, svc[i][1], r->hints);
+        if (retry_status < 0 || getaddrinfo_cb == NULL) {
+          // The callback will not be called
+          gpr_free(req);
+        }
+        return retry_status;
+      }
+    }
+  }
+  /* If this function calls uv_getaddrinfo, it will return that function's
+     return value. That function only returns numbers <=0, so we can safely
+     return 1 to indicate that we never retried */
+  return 1;
+}
+
 static grpc_error *handle_addrinfo_result(int status, struct addrinfo *result,
                                           grpc_resolved_addresses **addresses) {
   struct addrinfo *resp;
@@ -97,13 +126,21 @@ static void getaddrinfo_callback(uv_getaddrinfo_t *req, int status,
   request *r = (request *)req->data;
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   grpc_error *error;
+  int retry_status;
+
+  gpr_free(req);
+  retry_status = retry_named_port_failure(status, r, getaddrinfo_callback);
+  if (retry_status == 0) {
+    // The request is being retried. Nothing should be done here
+    return;
+  }
+  /* Either no retry was attempted, or the retry failed. Either way, the
+     original error probably has more interesting information */
   error = handle_addrinfo_result(status, res, r->addresses);
   grpc_closure_sched(&exec_ctx, r->on_done, error);
   grpc_exec_ctx_finish(&exec_ctx);
-
   gpr_free(r->hints);
   gpr_free(r);
-  gpr_free(req);
   uv_freeaddrinfo(res);
 }
 
@@ -143,6 +180,7 @@ static grpc_error *blocking_resolve_address_impl(
   uv_getaddrinfo_t req;
   int s;
   grpc_error *err;
+  int retry_status;
 
   req.addrinfo = NULL;
 
@@ -158,6 +196,12 @@ static grpc_error *blocking_resolve_address_impl(
   hints.ai_flags = AI_PASSIVE;     /* for wildcard IP address */
 
   s = uv_getaddrinfo(uv_default_loop(), &req, NULL, host, port, &hints);
+  request r = {
+      .addresses = addresses, .hints = &hints, .host = host, .port = port};
+  retry_status = retry_named_port_failure(s, &r, NULL);
+  if (retry_status <= 0) {
+    s = retry_status;
+  }
   err = handle_addrinfo_result(s, req.addrinfo, addresses);
 
 done:
@@ -200,6 +244,8 @@ static void resolve_address_impl(grpc_exec_ctx *exec_ctx, const char *name,
   r = gpr_malloc(sizeof(request));
   r->on_done = on_done;
   r->addresses = addrs;
+  r->host = host;
+  r->port = port;
   req = gpr_malloc(sizeof(uv_getaddrinfo_t));
   req->data = r;
 
@@ -222,6 +268,8 @@ static void resolve_address_impl(grpc_exec_ctx *exec_ctx, const char *name,
     gpr_free(r);
     gpr_free(req);
     gpr_free(hints);
+    gpr_free(host);
+    gpr_free(port);
   }
 }
 
diff --git a/src/core/lib/iomgr/tcp_client_uv.c b/src/core/lib/iomgr/tcp_client_uv.c
index ae66577caf..618483d9cb 100644
--- a/src/core/lib/iomgr/tcp_client_uv.c
+++ b/src/core/lib/iomgr/tcp_client_uv.c
@@ -76,7 +76,6 @@ static void uv_tc_on_alarm(grpc_exec_ctx *exec_ctx, void *acp,
     const char *str = grpc_error_string(error);
     gpr_log(GPR_DEBUG, "CLIENT_CONNECT: %s: on_alarm: error=%s",
             connect->addr_name, str);
-    grpc_error_free_string(str);
   }
   if (error == GRPC_ERROR_NONE) {
     /* error == NONE implies that the timer ran out, and wasn't cancelled. If
diff --git a/src/core/lib/iomgr/udp_server.c b/src/core/lib/iomgr/udp_server.c
index d1bcd89af1..71e295770a 100644
--- a/src/core/lib/iomgr/udp_server.c
+++ b/src/core/lib/iomgr/udp_server.c
@@ -109,8 +109,8 @@ struct grpc_udp_server {
   grpc_pollset **pollsets;
   /* number of pollsets in the pollsets array */
   size_t pollset_count;
-  /* The parent grpc server */
-  grpc_server *grpc_server;
+  /* opaque object to pass to callbacks */
+  void *user_data;
 };
 
 grpc_udp_server *grpc_udp_server_create(void) {
@@ -178,7 +178,7 @@ static void deactivated_all_ports(grpc_exec_ctx *exec_ctx, grpc_udp_server *s) {
       /* Call the orphan_cb to signal that the FD is about to be closed and
        * should no longer be used. */
       GPR_ASSERT(sp->orphan_cb);
-      sp->orphan_cb(exec_ctx, sp->emfd);
+      sp->orphan_cb(exec_ctx, sp->emfd, sp->server->user_data);
 
       grpc_fd_orphan(exec_ctx, sp->emfd, &sp->destroyed_closure, NULL,
                      "udp_listener_shutdown");
@@ -204,7 +204,7 @@ void grpc_udp_server_destroy(grpc_exec_ctx *exec_ctx, grpc_udp_server *s,
   if (s->active_ports) {
     for (sp = s->head; sp; sp = sp->next) {
       GPR_ASSERT(sp->orphan_cb);
-      sp->orphan_cb(exec_ctx, sp->emfd);
+      sp->orphan_cb(exec_ctx, sp->emfd, sp->server->user_data);
       grpc_fd_shutdown(exec_ctx, sp->emfd,
                        GRPC_ERROR_CREATE("Server destroyed"));
     }
@@ -299,7 +299,7 @@ static void on_read(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
 
   /* Tell the registered callback that data is available to read. */
   GPR_ASSERT(sp->read_cb);
-  sp->read_cb(exec_ctx, sp->emfd, sp->server->grpc_server);
+  sp->read_cb(exec_ctx, sp->emfd, sp->server->user_data);
 
   /* Re-arm the notification event so we get another chance to read. */
   grpc_fd_notify_on_read(exec_ctx, sp->emfd, &sp->read_closure);
@@ -322,7 +322,7 @@ static void on_write(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
 
   /* Tell the registered callback that the socket is writeable. */
   GPR_ASSERT(sp->write_cb);
-  sp->write_cb(exec_ctx, sp->emfd);
+  sp->write_cb(exec_ctx, sp->emfd, sp->server->user_data);
 
   /* Re-arm the notification event so we get another chance to write. */
   grpc_fd_notify_on_write(exec_ctx, sp->emfd, &sp->write_closure);
@@ -464,13 +464,13 @@ int grpc_udp_server_get_fd(grpc_udp_server *s, unsigned port_index) {
 
 void grpc_udp_server_start(grpc_exec_ctx *exec_ctx, grpc_udp_server *s,
                            grpc_pollset **pollsets, size_t pollset_count,
-                           grpc_server *server) {
+                           void *user_data) {
   size_t i;
   gpr_mu_lock(&s->mu);
   grpc_udp_listener *sp;
   GPR_ASSERT(s->active_ports == 0);
   s->pollsets = pollsets;
-  s->grpc_server = server;
+  s->user_data = user_data;
 
   sp = s->head;
   while (sp != NULL) {
diff --git a/src/core/lib/iomgr/udp_server.h b/src/core/lib/iomgr/udp_server.h
index ed63fa7d81..90842a47f0 100644
--- a/src/core/lib/iomgr/udp_server.h
+++ b/src/core/lib/iomgr/udp_server.h
@@ -47,23 +47,23 @@ typedef struct grpc_udp_server grpc_udp_server;
 
 /* Called when data is available to read from the socket. */
 typedef void (*grpc_udp_server_read_cb)(grpc_exec_ctx *exec_ctx, grpc_fd *emfd,
-                                        struct grpc_server *server);
+                                        void *user_data);
 
 /* Called when the socket is writeable. */
-typedef void (*grpc_udp_server_write_cb)(grpc_exec_ctx *exec_ctx,
-                                         grpc_fd *emfd);
+typedef void (*grpc_udp_server_write_cb)(grpc_exec_ctx *exec_ctx, grpc_fd *emfd,
+                                         void *user_data);
 
 /* Called when the grpc_fd is about to be orphaned (and the FD closed). */
 typedef void (*grpc_udp_server_orphan_cb)(grpc_exec_ctx *exec_ctx,
-                                          grpc_fd *emfd);
+                                          grpc_fd *emfd, void *user_data);
 
 /* Create a server, initially not bound to any ports */
 grpc_udp_server *grpc_udp_server_create(void);
 
-/* Start listening to bound ports */
+/* Start listening to bound ports. user_data is passed to callbacks. */
 void grpc_udp_server_start(grpc_exec_ctx *exec_ctx, grpc_udp_server *udp_server,
                            grpc_pollset **pollsets, size_t pollset_count,
-                           struct grpc_server *server);
+                           void *user_data);
 
 int grpc_udp_server_get_fd(grpc_udp_server *s, unsigned port_index);