Merge github.com:grpc/grpc into faster_timer_pool

5 files changed, 292 insertions, 10 deletions

diff --git a/test/core/census/intrusive_hash_map_test.c b/test/core/census/intrusive_hash_map_test.c
index fe8d3a1675..552546f9a3 100644
--- a/test/core/census/intrusive_hash_map_test.c
+++ b/test/core/census/intrusive_hash_map_test.c
@@ -49,7 +49,7 @@ static const uint32_t kInitialLog2Size = 4;
 typedef struct object { uint64_t val; } object;
 
 /* Helper function to allocate and initialize object. */
-static inline object *make_new_object(uint64_t val) {
+static __inline object *make_new_object(uint64_t val) {
   object *obj = (object *)gpr_malloc(sizeof(object));
   obj->val = val;
   return obj;
@@ -63,7 +63,7 @@ typedef struct ptr_item {
 
 /* Helper function that creates a new hash map item.  It is up to the user to
  * free the item that was allocated. */
-static inline ptr_item *make_ptr_item(uint64_t key, uint64_t value) {
+static __inline ptr_item *make_ptr_item(uint64_t key, uint64_t value) {
   ptr_item *new_item = (ptr_item *)gpr_malloc(sizeof(ptr_item));
   new_item->IHM_key = key;
   new_item->IHM_hash_link = NULL;
diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c
index 87ad095170..7614696cae 100644
--- a/test/core/surface/concurrent_connectivity_test.c
+++ b/test/core/surface/concurrent_connectivity_test.c
@@ -61,6 +61,14 @@
 #define DELAY_MILLIS 10
 #define POLL_MILLIS 3000
 
+#define NUM_OUTER_LOOPS_SHORT_TIMEOUTS 10
+#define NUM_INNER_LOOPS_SHORT_TIMEOUTS 100
+#define DELAY_MILLIS_SHORT_TIMEOUTS 1
+// in a successful test run, POLL_MILLIS should never be reached beause all runs
+// should
+// end after the shorter delay_millis
+#define POLL_MILLIS_SHORT_TIMEOUTS 30000
+
 static void *tag(int n) { return (void *)(uintptr_t)n; }
 static int detag(void *p) { return (int)(uintptr_t)p; }
 
@@ -79,6 +87,8 @@ void create_loop_destroy(void *addr) {
           grpc_timeout_milliseconds_to_deadline(POLL_MILLIS);
       GPR_ASSERT(grpc_completion_queue_next(cq, poll_time, NULL).type ==
                  GRPC_OP_COMPLETE);
+      /* check that the watcher from "watch state" was free'd */
+      GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(chan) == 0);
     }
     grpc_channel_destroy(chan);
     grpc_completion_queue_destroy(cq);
@@ -168,11 +178,10 @@ static void done_pollset_shutdown(grpc_exec_ctx *exec_ctx, void *pollset,
   gpr_free(pollset);
 }
 
-int main(int argc, char **argv) {
+int run_concurrent_connectivity_test() {
   struct server_thread_args args;
   memset(&args, 0, sizeof(args));
 
-  grpc_test_init(argc, argv);
   grpc_init();
 
   gpr_thd_id threads[NUM_THREADS];
@@ -242,3 +251,59 @@ int main(int argc, char **argv) {
   grpc_shutdown();
   return 0;
 }
+
+void watches_with_short_timeouts(void *addr) {
+  for (int i = 0; i < NUM_OUTER_LOOPS_SHORT_TIMEOUTS; ++i) {
+    grpc_completion_queue *cq = grpc_completion_queue_create_for_next(NULL);
+    grpc_channel *chan = grpc_insecure_channel_create((char *)addr, NULL, NULL);
+
+    for (int j = 0; j < NUM_INNER_LOOPS_SHORT_TIMEOUTS; ++j) {
+      gpr_timespec later_time =
+          grpc_timeout_milliseconds_to_deadline(DELAY_MILLIS_SHORT_TIMEOUTS);
+      grpc_connectivity_state state =
+          grpc_channel_check_connectivity_state(chan, 0);
+      GPR_ASSERT(state == GRPC_CHANNEL_IDLE);
+      grpc_channel_watch_connectivity_state(chan, state, later_time, cq, NULL);
+      gpr_timespec poll_time =
+          grpc_timeout_milliseconds_to_deadline(POLL_MILLIS_SHORT_TIMEOUTS);
+      grpc_event ev = grpc_completion_queue_next(cq, poll_time, NULL);
+      GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
+      GPR_ASSERT(ev.success == false);
+      /* check that the watcher from "watch state" was free'd */
+      GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(chan) == 0);
+    }
+    grpc_channel_destroy(chan);
+    grpc_completion_queue_destroy(cq);
+  }
+}
+
+// This test tries to catch deadlock situations.
+// With short timeouts on "watches" and long timeouts on cq next calls,
+// so that a QUEUE_TIMEOUT likely means that something is stuck.
+int run_concurrent_watches_with_short_timeouts_test() {
+  grpc_init();
+
+  gpr_thd_id threads[NUM_THREADS];
+
+  char *localhost = gpr_strdup("localhost:54321");
+  gpr_thd_options options = gpr_thd_options_default();
+  gpr_thd_options_set_joinable(&options);
+
+  for (size_t i = 0; i < NUM_THREADS; ++i) {
+    gpr_thd_new(&threads[i], watches_with_short_timeouts, localhost, &options);
+  }
+  for (size_t i = 0; i < NUM_THREADS; ++i) {
+    gpr_thd_join(threads[i]);
+  }
+  gpr_free(localhost);
+
+  grpc_shutdown();
+  return 0;
+}
+
+int main(int argc, char **argv) {
+  grpc_test_init(argc, argv);
+
+  run_concurrent_connectivity_test();
+  run_concurrent_watches_with_short_timeouts_test();
+}
diff --git a/test/core/surface/num_external_connectivity_watchers_test.c b/test/core/surface/num_external_connectivity_watchers_test.c
new file mode 100644
index 0000000000..93944c9ad5
--- /dev/null
+++ b/test/core/surface/num_external_connectivity_watchers_test.c
@@ -0,0 +1,214 @@
+/*
+ *
+ * Copyright 2016, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include <grpc/grpc.h>
+#include <grpc/grpc_security.h>
+#include <grpc/support/alloc.h>
+#include <grpc/support/host_port.h>
+#include <grpc/support/log.h>
+#include <grpc/support/thd.h>
+
+#include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/iomgr/exec_ctx.h"
+#include "test/core/end2end/data/ssl_test_data.h"
+#include "test/core/util/port.h"
+#include "test/core/util/test_config.h"
+
+typedef struct test_fixture {
+  const char *name;
+  grpc_channel *(*create_channel)(const char *addr);
+} test_fixture;
+
+static size_t next_tag = 1;
+
+static void channel_idle_start_watch(grpc_channel *channel,
+                                     grpc_completion_queue *cq) {
+  gpr_timespec connect_deadline = grpc_timeout_milliseconds_to_deadline(1);
+  GPR_ASSERT(grpc_channel_check_connectivity_state(channel, 0) ==
+             GRPC_CHANNEL_IDLE);
+
+  grpc_channel_watch_connectivity_state(
+      channel, GRPC_CHANNEL_IDLE, connect_deadline, cq, (void *)(next_tag++));
+  gpr_log(GPR_DEBUG, "number of active connect watchers: %d",
+          grpc_channel_num_external_connectivity_watchers(channel));
+}
+
+static void channel_idle_poll_for_timeout(grpc_channel *channel,
+                                          grpc_completion_queue *cq) {
+  grpc_event ev =
+      grpc_completion_queue_next(cq, gpr_inf_future(GPR_CLOCK_REALTIME), NULL);
+
+  /* expect watch_connectivity_state to end with a timeout */
+  GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
+  GPR_ASSERT(ev.success == false);
+  GPR_ASSERT(grpc_channel_check_connectivity_state(channel, 0) ==
+             GRPC_CHANNEL_IDLE);
+}
+
+/* Test and use the "num_external_watchers" call to make sure
+ * that "connectivity watcher" structs are free'd just after, if
+ * their corresponding timeouts occur. */
+static void run_timeouts_test(const test_fixture *fixture) {
+  gpr_log(GPR_INFO, "TEST: %s", fixture->name);
+
+  char *addr;
+
+  grpc_init();
+
+  gpr_join_host_port(&addr, "localhost", grpc_pick_unused_port_or_die());
+
+  grpc_channel *channel = fixture->create_channel(addr);
+  grpc_completion_queue *cq = grpc_completion_queue_create_for_next(NULL);
+
+  /* start 1 watcher and then let it time out */
+  channel_idle_start_watch(channel, cq);
+  channel_idle_poll_for_timeout(channel, cq);
+  GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(channel) == 0);
+
+  /* start 3 watchers and then let them all time out */
+  for (size_t i = 1; i <= 3; i++) {
+    channel_idle_start_watch(channel, cq);
+  }
+  for (size_t i = 1; i <= 3; i++) {
+    channel_idle_poll_for_timeout(channel, cq);
+  }
+  GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(channel) == 0);
+
+  /* start 3 watchers, see one time out, start another 3, and then see them all
+   * time out */
+  for (size_t i = 1; i <= 3; i++) {
+    channel_idle_start_watch(channel, cq);
+  }
+  channel_idle_poll_for_timeout(channel, cq);
+  for (size_t i = 3; i <= 5; i++) {
+    channel_idle_start_watch(channel, cq);
+  }
+  for (size_t i = 1; i <= 5; i++) {
+    channel_idle_poll_for_timeout(channel, cq);
+  }
+  GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(channel) == 0);
+
+  grpc_channel_destroy(channel);
+  grpc_completion_queue_shutdown(cq);
+  GPR_ASSERT(
+      grpc_completion_queue_next(cq, gpr_inf_future(GPR_CLOCK_REALTIME), NULL)
+          .type == GRPC_QUEUE_SHUTDOWN);
+  grpc_completion_queue_destroy(cq);
+
+  grpc_shutdown();
+  gpr_free(addr);
+}
+
+/* An edge scenario; sets channel state to explicitly, and outside
+ * of a polling call. */
+static void run_channel_shutdown_before_timeout_test(
+    const test_fixture *fixture) {
+  gpr_log(GPR_INFO, "TEST: %s", fixture->name);
+
+  char *addr;
+
+  grpc_init();
+
+  gpr_join_host_port(&addr, "localhost", grpc_pick_unused_port_or_die());
+
+  grpc_channel *channel = fixture->create_channel(addr);
+  grpc_completion_queue *cq = grpc_completion_queue_create_for_next(NULL);
+
+  /* start 1 watcher and then shut down the channel before the timer goes off */
+  GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(channel) == 0);
+
+  /* expecting a 30 second timeout to go off much later than the shutdown. */
+  gpr_timespec connect_deadline = grpc_timeout_seconds_to_deadline(30);
+  GPR_ASSERT(grpc_channel_check_connectivity_state(channel, 0) ==
+             GRPC_CHANNEL_IDLE);
+
+  grpc_channel_watch_connectivity_state(channel, GRPC_CHANNEL_IDLE,
+                                        connect_deadline, cq, (void *)1);
+  grpc_channel_destroy(channel);
+
+  grpc_event ev =
+      grpc_completion_queue_next(cq, gpr_inf_future(GPR_CLOCK_REALTIME), NULL);
+  GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
+  /* expect success with a state transition to CHANNEL_SHUTDOWN */
+  GPR_ASSERT(ev.success == true);
+
+  grpc_completion_queue_shutdown(cq);
+  GPR_ASSERT(
+      grpc_completion_queue_next(cq, gpr_inf_future(GPR_CLOCK_REALTIME), NULL)
+          .type == GRPC_QUEUE_SHUTDOWN);
+  grpc_completion_queue_destroy(cq);
+
+  grpc_shutdown();
+  gpr_free(addr);
+}
+
+static grpc_channel *insecure_test_create_channel(const char *addr) {
+  return grpc_insecure_channel_create(addr, NULL, NULL);
+}
+
+static const test_fixture insecure_test = {
+    "insecure", insecure_test_create_channel,
+};
+
+static grpc_channel *secure_test_create_channel(const char *addr) {
+  grpc_channel_credentials *ssl_creds =
+      grpc_ssl_credentials_create(test_root_cert, NULL, NULL);
+  grpc_arg ssl_name_override = {GRPC_ARG_STRING,
+                                GRPC_SSL_TARGET_NAME_OVERRIDE_ARG,
+                                {"foo.test.google.fr"}};
+  grpc_channel_args *new_client_args =
+      grpc_channel_args_copy_and_add(NULL, &ssl_name_override, 1);
+  grpc_channel *channel =
+      grpc_secure_channel_create(ssl_creds, addr, new_client_args, NULL);
+  {
+    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
+    grpc_channel_args_destroy(&exec_ctx, new_client_args);
+    grpc_exec_ctx_finish(&exec_ctx);
+  }
+  grpc_channel_credentials_release(ssl_creds);
+  return channel;
+}
+
+static const test_fixture secure_test = {
+    "secure", secure_test_create_channel,
+};
+
+int main(int argc, char **argv) {
+  grpc_test_init(argc, argv);
+
+  run_timeouts_test(&insecure_test);
+  run_timeouts_test(&secure_test);
+
+  run_channel_shutdown_before_timeout_test(&insecure_test);
+  run_channel_shutdown_before_timeout_test(&secure_test);
+}
diff --git a/test/core/surface/sequential_connectivity_test.c b/test/core/surface/sequential_connectivity_test.c
index fbecdd7e38..a0f18e9bbf 100644
--- a/test/core/surface/sequential_connectivity_test.c
+++ b/test/core/surface/sequential_connectivity_test.c
@@ -100,6 +100,9 @@ static void run_test(const test_fixture *fixture) {
                                             connect_deadline, cq, NULL);
       grpc_event ev = grpc_completion_queue_next(
           cq, gpr_inf_future(GPR_CLOCK_REALTIME), NULL);
+      /* check that the watcher from "watch state" was free'd */
+      GPR_ASSERT(grpc_channel_num_external_connectivity_watchers(channels[i]) ==
+                 0);
       GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
       GPR_ASSERT(ev.tag == NULL);
       GPR_ASSERT(ev.success == true);
diff --git a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc
index d7e3a9cf47..9f616fe152 100644
--- a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc
+++ b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc
@@ -419,18 +419,18 @@ static void BM_PumpUnbalancedUnary_Trickle(benchmark::State& state) {
 }
 
 static void UnaryTrickleArgs(benchmark::internal::Benchmark* b) {
+  // A selection of interesting numbers
   const int cli_1024k = 1024 * 1024;
   const int cli_32M = 32 * 1024 * 1024;
   const int svr_256k = 256 * 1024;
   const int svr_4M = 4 * 1024 * 1024;
   const int svr_64M = 64 * 1024 * 1024;
   for (int bw = 64; bw <= 128 * 1024 * 1024; bw *= 16) {
-    b->Args({bw, cli_1024k, svr_256k});
-    b->Args({bw, cli_1024k, svr_4M});
-    b->Args({bw, cli_1024k, svr_64M});
-    b->Args({bw, cli_32M, svr_256k});
-    b->Args({bw, cli_32M, svr_4M});
-    b->Args({bw, cli_32M, svr_64M});
+    for (auto svr : {svr_256k, svr_4M, svr_64M}) {
+      for (auto cli : {cli_1024k, cli_32M}) {
+        b->Args({cli, svr, bw});
+      }
+    }
   }
 }
 BENCHMARK(BM_PumpUnbalancedUnary_Trickle)->Apply(UnaryTrickleArgs);