From c5b3b25f8d94404a6069cacd756be47d8e9d8de3 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Wed, 22 Feb 2017 11:25:10 -0800 Subject: [PATCH 01/14] Change ssl_server_fuzzer.c to use ssl_test_data.h instead of loading mock SSL data from file --- test/core/security/BUILD | 2 +- test/core/security/ssl_server_fuzzer.c | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test/core/security/BUILD b/test/core/security/BUILD index e750c39b7c6..1cb03c5cfe7 100644 --- a/test/core/security/BUILD +++ b/test/core/security/BUILD @@ -34,7 +34,7 @@ load("//test/core/util:grpc_fuzzer.bzl", "grpc_fuzzer") grpc_fuzzer( name = "ssl_server_fuzzer", srcs = ["ssl_server_fuzzer.c"], - deps = ["//:gpr", "//:grpc", "//test/core/util:grpc_test_util"], + deps = ["//:gpr", "//:grpc", "//test/core/util:grpc_test_util", "//test/core/end2end:ssl_test_data"], corpus = "corpus", copts = ["-std=c99"], ) diff --git a/test/core/security/ssl_server_fuzzer.c b/test/core/security/ssl_server_fuzzer.c index f789278add8..fe98799144f 100644 --- a/test/core/security/ssl_server_fuzzer.c +++ b/test/core/security/ssl_server_fuzzer.c @@ -38,6 +38,7 @@ #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/security/credentials/credentials.h" #include "src/core/lib/security/transport/security_connector.h" +#include "test/core/end2end/data/ssl_test_data.h" #include "test/core/util/memory_counters.h" #include "test/core/util/mock_endpoint.h" @@ -50,6 +51,7 @@ bool leak_check = false; #define SSL_KEY_PATH "src/core/lib/tsi/test_creds/server1.key" #define SSL_CA_PATH "src/core/lib/tsi/test_creds/ca.pem" + static void discard_write(grpc_slice slice) {} static void dont_log(gpr_log_func_args *args) {} @@ -88,12 +90,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { // Load key pair and establish server SSL credentials. grpc_ssl_pem_key_cert_pair pem_key_cert_pair; grpc_slice ca_slice, cert_slice, key_slice; - GPR_ASSERT(GRPC_LOG_IF_ERROR("load_file", - grpc_load_file(SSL_CA_PATH, 1, &ca_slice))); - GPR_ASSERT(GRPC_LOG_IF_ERROR("load_file", - grpc_load_file(SSL_CERT_PATH, 1, &cert_slice))); - GPR_ASSERT(GRPC_LOG_IF_ERROR("load_file", - grpc_load_file(SSL_KEY_PATH, 1, &key_slice))); + ca_slice = grpc_slice_from_static_string(test_root_cert); + cert_slice = grpc_slice_from_static_string(test_server1_cert); + key_slice = grpc_slice_from_static_string(test_server1_key); const char *ca_cert = (const char *)GRPC_SLICE_START_PTR(ca_slice); pem_key_cert_pair.private_key = (const char *)GRPC_SLICE_START_PTR(key_slice); pem_key_cert_pair.cert_chain = (const char *)GRPC_SLICE_START_PTR(cert_slice); From d9b257a1547f8d3d6ddf66036e19561e64de6531 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Wed, 22 Feb 2017 15:14:28 -0800 Subject: [PATCH 02/14] Add fuzzer options for oss-fuzz --- test/core/security/ssl_server_fuzzer.c | 5 ----- tools/fuzzer/options/api_fuzzer.options | 3 +++ tools/fuzzer/options/client_fuzzer.options | 3 +++ tools/fuzzer/options/fuzzer.options | 2 ++ tools/fuzzer/options/fuzzer_response.options | 2 ++ tools/fuzzer/options/fuzzer_serverlist.options | 2 ++ tools/fuzzer/options/hpack_parser_fuzzer_test.options | 3 +++ tools/fuzzer/options/percent_decode_fuzzer.options | 2 ++ tools/fuzzer/options/percent_encode_fuzzer.options | 2 ++ tools/fuzzer/options/request_fuzzer.options | 3 +++ tools/fuzzer/options/response_fuzzer.options | 3 +++ tools/fuzzer/options/server_fuzzer.options | 3 +++ tools/fuzzer/options/ssl_server_fuzzer.options | 2 ++ tools/fuzzer/options/uri_fuzzer_test.options | 2 ++ 14 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 tools/fuzzer/options/api_fuzzer.options create mode 100644 tools/fuzzer/options/client_fuzzer.options create mode 100644 tools/fuzzer/options/fuzzer.options create mode 100644 tools/fuzzer/options/fuzzer_response.options create mode 100644 tools/fuzzer/options/fuzzer_serverlist.options create mode 100644 tools/fuzzer/options/hpack_parser_fuzzer_test.options create mode 100644 tools/fuzzer/options/percent_decode_fuzzer.options create mode 100644 tools/fuzzer/options/percent_encode_fuzzer.options create mode 100644 tools/fuzzer/options/request_fuzzer.options create mode 100644 tools/fuzzer/options/response_fuzzer.options create mode 100644 tools/fuzzer/options/server_fuzzer.options create mode 100644 tools/fuzzer/options/ssl_server_fuzzer.options create mode 100644 tools/fuzzer/options/uri_fuzzer_test.options diff --git a/test/core/security/ssl_server_fuzzer.c b/test/core/security/ssl_server_fuzzer.c index fe98799144f..7a3612c419b 100644 --- a/test/core/security/ssl_server_fuzzer.c +++ b/test/core/security/ssl_server_fuzzer.c @@ -47,11 +47,6 @@ bool squelch = true; // Turning this on will fail the leak check. bool leak_check = false; -#define SSL_CERT_PATH "src/core/lib/tsi/test_creds/server1.pem" -#define SSL_KEY_PATH "src/core/lib/tsi/test_creds/server1.key" -#define SSL_CA_PATH "src/core/lib/tsi/test_creds/ca.pem" - - static void discard_write(grpc_slice slice) {} static void dont_log(gpr_log_func_args *args) {} diff --git a/tools/fuzzer/options/api_fuzzer.options b/tools/fuzzer/options/api_fuzzer.options new file mode 100644 index 00000000000..8871ae21b6a --- /dev/null +++ b/tools/fuzzer/options/api_fuzzer.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 2048 +dict = api_fuzzer.dictionary diff --git a/tools/fuzzer/options/client_fuzzer.options b/tools/fuzzer/options/client_fuzzer.options new file mode 100644 index 00000000000..fd2eebf7d25 --- /dev/null +++ b/tools/fuzzer/options/client_fuzzer.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 2048 +dict = hpack.dictionary diff --git a/tools/fuzzer/options/fuzzer.options b/tools/fuzzer/options/fuzzer.options new file mode 100644 index 00000000000..5d468bc6e48 --- /dev/null +++ b/tools/fuzzer/options/fuzzer.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 512 diff --git a/tools/fuzzer/options/fuzzer_response.options b/tools/fuzzer/options/fuzzer_response.options new file mode 100644 index 00000000000..5dcdfac7a69 --- /dev/null +++ b/tools/fuzzer/options/fuzzer_response.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 128 diff --git a/tools/fuzzer/options/fuzzer_serverlist.options b/tools/fuzzer/options/fuzzer_serverlist.options new file mode 100644 index 00000000000..5dcdfac7a69 --- /dev/null +++ b/tools/fuzzer/options/fuzzer_serverlist.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 128 diff --git a/tools/fuzzer/options/hpack_parser_fuzzer_test.options b/tools/fuzzer/options/hpack_parser_fuzzer_test.options new file mode 100644 index 00000000000..584487fafc2 --- /dev/null +++ b/tools/fuzzer/options/hpack_parser_fuzzer_test.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 512 +dict = hpack.dictionary diff --git a/tools/fuzzer/options/percent_decode_fuzzer.options b/tools/fuzzer/options/percent_decode_fuzzer.options new file mode 100644 index 00000000000..ea2785e1104 --- /dev/null +++ b/tools/fuzzer/options/percent_decode_fuzzer.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 32 diff --git a/tools/fuzzer/options/percent_encode_fuzzer.options b/tools/fuzzer/options/percent_encode_fuzzer.options new file mode 100644 index 00000000000..ea2785e1104 --- /dev/null +++ b/tools/fuzzer/options/percent_encode_fuzzer.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 32 diff --git a/tools/fuzzer/options/request_fuzzer.options b/tools/fuzzer/options/request_fuzzer.options new file mode 100644 index 00000000000..fd32ac16e1b --- /dev/null +++ b/tools/fuzzer/options/request_fuzzer.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 2048 + diff --git a/tools/fuzzer/options/response_fuzzer.options b/tools/fuzzer/options/response_fuzzer.options new file mode 100644 index 00000000000..fd32ac16e1b --- /dev/null +++ b/tools/fuzzer/options/response_fuzzer.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 2048 + diff --git a/tools/fuzzer/options/server_fuzzer.options b/tools/fuzzer/options/server_fuzzer.options new file mode 100644 index 00000000000..fd2eebf7d25 --- /dev/null +++ b/tools/fuzzer/options/server_fuzzer.options @@ -0,0 +1,3 @@ +[libfuzzer] +max_len = 2048 +dict = hpack.dictionary diff --git a/tools/fuzzer/options/ssl_server_fuzzer.options b/tools/fuzzer/options/ssl_server_fuzzer.options new file mode 100644 index 00000000000..60bd9b0b2fa --- /dev/null +++ b/tools/fuzzer/options/ssl_server_fuzzer.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 2048 diff --git a/tools/fuzzer/options/uri_fuzzer_test.options b/tools/fuzzer/options/uri_fuzzer_test.options new file mode 100644 index 00000000000..5dcdfac7a69 --- /dev/null +++ b/tools/fuzzer/options/uri_fuzzer_test.options @@ -0,0 +1,2 @@ +[libfuzzer] +max_len = 128 From d0084c220e3f3b5a35adbed430942237ef327323 Mon Sep 17 00:00:00 2001 From: yang-g Date: Mon, 6 Mar 2017 11:23:35 -0800 Subject: [PATCH 03/14] Include x.pb.h instead of x.grpc.pb.h for message-only protos --- test/cpp/interop/http2_client.cc | 2 +- test/cpp/interop/http2_client.h | 2 +- test/cpp/interop/interop_client.cc | 4 ++-- test/cpp/interop/interop_client.h | 2 +- test/cpp/interop/interop_server.cc | 4 ++-- test/cpp/interop/reconnect_interop_client.cc | 4 ++-- test/cpp/interop/reconnect_interop_server.cc | 4 ++-- test/cpp/qps/client.h | 2 +- test/cpp/qps/driver.h | 2 +- test/cpp/qps/histogram.h | 2 +- test/cpp/qps/server.h | 4 ++-- 11 files changed, 16 insertions(+), 16 deletions(-) diff --git a/test/cpp/interop/http2_client.cc b/test/cpp/interop/http2_client.cc index 01c07823cfe..38a437f39f8 100644 --- a/test/cpp/interop/http2_client.cc +++ b/test/cpp/interop/http2_client.cc @@ -41,7 +41,7 @@ #include #include "src/core/lib/transport/byte_stream.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" #include "test/cpp/interop/http2_client.h" diff --git a/test/cpp/interop/http2_client.h b/test/cpp/interop/http2_client.h index 12df5d26bc2..e57d695205b 100644 --- a/test/cpp/interop/http2_client.h +++ b/test/cpp/interop/http2_client.h @@ -38,7 +38,7 @@ #include #include -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" namespace grpc { diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc index b7f2723c39b..55ba324cc7d 100644 --- a/test/cpp/interop/interop_client.cc +++ b/test/cpp/interop/interop_client.cc @@ -46,8 +46,8 @@ #include #include "src/core/lib/transport/byte_stream.h" -#include "src/proto/grpc/testing/empty.grpc.pb.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/empty.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" #include "test/cpp/interop/client_helper.h" #include "test/cpp/interop/interop_client.h" diff --git a/test/cpp/interop/interop_client.h b/test/cpp/interop/interop_client.h index 74f4db6b789..efcb7d28608 100644 --- a/test/cpp/interop/interop_client.h +++ b/test/cpp/interop/interop_client.h @@ -38,7 +38,7 @@ #include #include -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" namespace grpc { diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index 5a810b45ef4..1cbca179287 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -48,8 +48,8 @@ #include "src/core/lib/support/string.h" #include "src/core/lib/transport/byte_stream.h" -#include "src/proto/grpc/testing/empty.grpc.pb.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/empty.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" #include "test/cpp/interop/server_helper.h" #include "test/cpp/util/test_config.h" diff --git a/test/cpp/interop/reconnect_interop_client.cc b/test/cpp/interop/reconnect_interop_client.cc index 1c2f6066377..01d985068dc 100644 --- a/test/cpp/interop/reconnect_interop_client.cc +++ b/test/cpp/interop/reconnect_interop_client.cc @@ -40,8 +40,8 @@ #include #include #include -#include "src/proto/grpc/testing/empty.grpc.pb.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/empty.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" #include "test/cpp/util/create_test_channel.h" #include "test/cpp/util/test_config.h" diff --git a/test/cpp/interop/reconnect_interop_server.cc b/test/cpp/interop/reconnect_interop_server.cc index 634d0a90fce..8d1b884af91 100644 --- a/test/cpp/interop/reconnect_interop_server.cc +++ b/test/cpp/interop/reconnect_interop_server.cc @@ -47,8 +47,8 @@ #include #include -#include "src/proto/grpc/testing/empty.grpc.pb.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/empty.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "src/proto/grpc/testing/test.grpc.pb.h" #include "test/core/util/reconnect_server.h" #include "test/cpp/util/test_config.h" diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index baa9304cc2d..25a19a5a740 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -46,7 +46,7 @@ #include #include -#include "src/proto/grpc/testing/payloads.grpc.pb.h" +#include "src/proto/grpc/testing/payloads.pb.h" #include "src/proto/grpc/testing/services.grpc.pb.h" #include "test/cpp/qps/histogram.h" diff --git a/test/cpp/qps/driver.h b/test/cpp/qps/driver.h index e72d30a4eff..dd32a16c879 100644 --- a/test/cpp/qps/driver.h +++ b/test/cpp/qps/driver.h @@ -36,7 +36,7 @@ #include -#include "src/proto/grpc/testing/control.grpc.pb.h" +#include "src/proto/grpc/testing/control.pb.h" #include "test/cpp/qps/histogram.h" namespace grpc { diff --git a/test/cpp/qps/histogram.h b/test/cpp/qps/histogram.h index acb415f0a10..470a3943013 100644 --- a/test/cpp/qps/histogram.h +++ b/test/cpp/qps/histogram.h @@ -35,7 +35,7 @@ #define TEST_QPS_HISTOGRAM_H #include -#include "src/proto/grpc/testing/stats.grpc.pb.h" +#include "src/proto/grpc/testing/stats.pb.h" namespace grpc { namespace testing { diff --git a/test/cpp/qps/server.h b/test/cpp/qps/server.h index 821d5935beb..8fbf37a0957 100644 --- a/test/cpp/qps/server.h +++ b/test/cpp/qps/server.h @@ -38,8 +38,8 @@ #include #include -#include "src/proto/grpc/testing/control.grpc.pb.h" -#include "src/proto/grpc/testing/messages.grpc.pb.h" +#include "src/proto/grpc/testing/control.pb.h" +#include "src/proto/grpc/testing/messages.pb.h" #include "test/core/end2end/data/ssl_test_data.h" #include "test/core/util/port.h" #include "test/cpp/qps/usage_timer.h" From e0341792d5a33469bbac481e9737bd00e3cb8900 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 8 Mar 2017 15:59:16 -0800 Subject: [PATCH 04/14] Remove most usage of the grpc_call lock --- src/core/lib/surface/call.c | 77 ++++++++++++++----------------------- 1 file changed, 29 insertions(+), 48 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index c2547c5147a..9b01c46bd59 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -143,9 +143,10 @@ struct grpc_call { grpc_channel *channel; grpc_call *parent; grpc_call *first_child; + gpr_atm has_children; gpr_timespec start_time; - /* TODO(ctiller): share with cq if possible? */ - gpr_mu mu; + /* protects first_child, setting has_children, and child next/prev links */ + gpr_mu child_list_mu; /* client or server call */ bool is_client; @@ -161,7 +162,7 @@ struct grpc_call { bool receiving_message; bool requested_final_op; bool received_final_op; - bool sent_any_op; + gpr_atm num_ops_sent; /* have we received initial metadata */ bool has_initial_md_been_received; @@ -275,7 +276,7 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, GPR_TIMER_BEGIN("grpc_call_create", 0); call = gpr_zalloc(sizeof(grpc_call) + channel_stack->call_stack_size); *out_call = call; - gpr_mu_init(&call->mu); + gpr_mu_init(&call->child_list_mu); call->channel = args->channel; call->cq = args->cq; call->parent = args->parent_call; @@ -313,7 +314,8 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, GPR_ASSERT(call->is_client); GPR_ASSERT(!args->parent_call->is_client); - gpr_mu_lock(&args->parent_call->mu); + gpr_mu_lock(&args->parent_call->child_list_mu); + gpr_atm_no_barrier_store(&args->parent_call->has_children, 1); if (args->propagation_mask & GRPC_PROPAGATE_DEADLINE) { send_deadline = gpr_time_min( @@ -353,7 +355,7 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, call; } - gpr_mu_unlock(&args->parent_call->mu); + gpr_mu_unlock(&args->parent_call->child_list_mu); } call->send_deadline = send_deadline; @@ -435,7 +437,7 @@ static void destroy_call(grpc_exec_ctx *exec_ctx, void *call, if (c->receiving_stream != NULL) { grpc_byte_stream_destroy(exec_ctx, c->receiving_stream); } - gpr_mu_destroy(&c->mu); + gpr_mu_destroy(&c->child_list_mu); for (ii = 0; ii < c->send_extra_metadata_count; ii++) { GRPC_MDELEM_UNREF(exec_ctx, c->send_extra_metadata[ii].md); } @@ -473,7 +475,7 @@ void grpc_call_destroy(grpc_call *c) { GRPC_API_TRACE("grpc_call_destroy(c=%p)", 1, (c)); if (parent) { - gpr_mu_lock(&parent->mu); + gpr_mu_lock(&parent->child_list_mu); if (c == parent->first_child) { parent->first_child = c->sibling_next; if (c == parent->first_child) { @@ -482,15 +484,13 @@ void grpc_call_destroy(grpc_call *c) { c->sibling_prev->sibling_next = c->sibling_next; c->sibling_next->sibling_prev = c->sibling_prev; } - gpr_mu_unlock(&parent->mu); + gpr_mu_unlock(&parent->child_list_mu); GRPC_CALL_INTERNAL_UNREF(&exec_ctx, parent, "child"); } - gpr_mu_lock(&c->mu); GPR_ASSERT(!c->destroy_called); c->destroy_called = 1; - cancel = c->sent_any_op && !c->received_final_op; - gpr_mu_unlock(&c->mu); + cancel = gpr_atm_no_barrier_load(&c->num_ops_sent) && !c->received_final_op; if (cancel) { cancel_with_error(&exec_ctx, c, STATUS_FROM_API_OVERRIDE, GRPC_ERROR_CANCELLED); @@ -555,10 +555,8 @@ grpc_call_error grpc_call_cancel_with_status(grpc_call *c, "c=%p, status=%d, description=%s, reserved=%p)", 4, (c, (int)status, description, reserved)); GPR_ASSERT(reserved == NULL); - gpr_mu_lock(&c->mu); cancel_with_status(&exec_ctx, c, STATUS_FROM_API_OVERRIDE, status, description); - gpr_mu_unlock(&c->mu); grpc_exec_ctx_finish(&exec_ctx); return GRPC_CALL_OK; } @@ -715,9 +713,7 @@ static void set_incoming_compression_algorithm( grpc_compression_algorithm grpc_call_test_only_get_compression_algorithm( grpc_call *call) { grpc_compression_algorithm algorithm; - gpr_mu_lock(&call->mu); algorithm = call->incoming_compression_algorithm; - gpr_mu_unlock(&call->mu); return algorithm; } @@ -729,9 +725,7 @@ static grpc_compression_algorithm compression_algorithm_for_level_locked( uint32_t grpc_call_test_only_get_message_flags(grpc_call *call) { uint32_t flags; - gpr_mu_lock(&call->mu); flags = call->test_only_last_message_flags; - gpr_mu_unlock(&call->mu); return flags; } @@ -785,9 +779,7 @@ static void set_encodings_accepted_by_peer(grpc_exec_ctx *exec_ctx, uint32_t grpc_call_test_only_get_encodings_accepted_by_peer(grpc_call *call) { uint32_t encodings_accepted_by_peer; - gpr_mu_lock(&call->mu); encodings_accepted_by_peer = call->encodings_accepted_by_peer; - gpr_mu_unlock(&call->mu); return encodings_accepted_by_peer; } @@ -1083,8 +1075,6 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, grpc_call *call = bctl->call; grpc_error *error = consolidate_batch_errors(bctl); - gpr_mu_lock(&call->mu); - if (bctl->send_initial_metadata) { grpc_metadata_batch_destroy( exec_ctx, @@ -1105,17 +1095,21 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, call->received_final_op = true; /* propagate cancellation to any interested children */ - child_call = call->first_child; - if (child_call != NULL) { - do { - next_child_call = child_call->sibling_next; - if (child_call->cancellation_is_inherited) { - GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); - grpc_call_cancel(child_call, NULL); - GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); - } - child_call = next_child_call; - } while (child_call != call->first_child); + if (gpr_atm_no_barrier_load(&call->has_children)) { + gpr_mu_lock(&call->child_list_mu); + child_call = call->first_child; + if (child_call != NULL) { + do { + next_child_call = child_call->sibling_next; + if (child_call->cancellation_is_inherited) { + GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); + grpc_call_cancel(child_call, NULL); + GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); + } + child_call = next_child_call; + } while (child_call != call->first_child); + } + gpr_mu_unlock(&call->child_list_mu); } if (call->is_client) { @@ -1130,7 +1124,6 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(error); error = GRPC_ERROR_NONE; } - gpr_mu_unlock(&call->mu); if (bctl->is_notify_tag_closure) { /* unrefs bctl->error */ @@ -1221,7 +1214,6 @@ static void receiving_stream_ready(grpc_exec_ctx *exec_ctx, void *bctlp, grpc_error *error) { batch_control *bctl = bctlp; grpc_call *call = bctl->call; - gpr_mu_lock(&bctl->call->mu); if (error != GRPC_ERROR_NONE) { if (call->receiving_stream != NULL) { grpc_byte_stream_destroy(exec_ctx, call->receiving_stream); @@ -1233,11 +1225,9 @@ static void receiving_stream_ready(grpc_exec_ctx *exec_ctx, void *bctlp, } if (call->has_initial_md_been_received || error != GRPC_ERROR_NONE || call->receiving_stream == NULL) { - gpr_mu_unlock(&bctl->call->mu); process_data_after_md(exec_ctx, bctlp); } else { call->saved_receiving_stream_ready_bctlp = bctlp; - gpr_mu_unlock(&bctl->call->mu); } } @@ -1309,8 +1299,6 @@ static void receiving_initial_metadata_ready(grpc_exec_ctx *exec_ctx, batch_control *bctl = bctlp; grpc_call *call = bctl->call; - gpr_mu_lock(&call->mu); - add_batch_error(exec_ctx, bctl, GRPC_ERROR_REF(error), false); if (error == GRPC_ERROR_NONE) { grpc_metadata_batch *md = @@ -1336,11 +1324,9 @@ static void receiving_initial_metadata_ready(grpc_exec_ctx *exec_ctx, receiving_stream_ready, call->saved_receiving_stream_ready_bctlp, grpc_schedule_on_exec_ctx); call->saved_receiving_stream_ready_bctlp = NULL; - grpc_closure_sched(exec_ctx, saved_rsr_closure, GRPC_ERROR_REF(error)); + grpc_closure_run(exec_ctx, saved_rsr_closure, GRPC_ERROR_REF(error)); } - gpr_mu_unlock(&call->mu); - finish_batch_step(exec_ctx, bctl); } @@ -1393,7 +1379,6 @@ static grpc_call_error call_start_batch(grpc_exec_ctx *exec_ctx, bctl->notify_tag = notify_tag; bctl->is_notify_tag_closure = (uint8_t)(is_notify_tag_closure != 0); - gpr_mu_lock(&call->mu); grpc_transport_stream_op *stream_op = &bctl->op; memset(stream_op, 0, sizeof(*stream_op)); stream_op->covered_by_poller = true; @@ -1679,8 +1664,7 @@ static grpc_call_error call_start_batch(grpc_exec_ctx *exec_ctx, grpc_closure_init(&bctl->finish_batch, finish_batch, bctl, grpc_schedule_on_exec_ctx); stream_op->on_complete = &bctl->finish_batch; - call->sent_any_op = true; - gpr_mu_unlock(&call->mu); + gpr_atm_no_barrier_fetch_add(&call->num_ops_sent, 1); execute_op(exec_ctx, call, stream_op); @@ -1711,7 +1695,6 @@ done_with_error: if (bctl->recv_final_op) { call->requested_final_op = 0; } - gpr_mu_unlock(&call->mu); goto done; } @@ -1760,10 +1743,8 @@ uint8_t grpc_call_is_client(grpc_call *call) { return call->is_client; } grpc_compression_algorithm grpc_call_compression_for_level( grpc_call *call, grpc_compression_level level) { - gpr_mu_lock(&call->mu); grpc_compression_algorithm algo = compression_algorithm_for_level_locked(call, level); - gpr_mu_unlock(&call->mu); return algo; } From 676db291d2387603f795dd6bd5853188e0ff7f63 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 8 Mar 2017 16:10:56 -0800 Subject: [PATCH 05/14] Fix barriers --- src/core/lib/surface/call.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 9b01c46bd59..2f3ddd6bcb9 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -315,7 +315,7 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, GPR_ASSERT(!args->parent_call->is_client); gpr_mu_lock(&args->parent_call->child_list_mu); - gpr_atm_no_barrier_store(&args->parent_call->has_children, 1); + gpr_atm_rel_store(&args->parent_call->has_children, 1); if (args->propagation_mask & GRPC_PROPAGATE_DEADLINE) { send_deadline = gpr_time_min( @@ -1095,7 +1095,7 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, call->received_final_op = true; /* propagate cancellation to any interested children */ - if (gpr_atm_no_barrier_load(&call->has_children)) { + if (gpr_atm_acq_load(&call->has_children)) { gpr_mu_lock(&call->child_list_mu); child_call = call->first_child; if (child_call != NULL) { From b597dcf53a5057425ba24a7c8dbed2f65f8c31f4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 9 Mar 2017 07:02:11 -0800 Subject: [PATCH 06/14] Race fixes --- src/core/lib/surface/call.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 2f3ddd6bcb9..eafeef0a207 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -161,9 +161,9 @@ struct grpc_call { bool received_initial_metadata; bool receiving_message; bool requested_final_op; - bool received_final_op; - gpr_atm num_ops_sent; - + gpr_atm any_ops_sent_atm; + gpr_atm received_final_op_atm; + /* have we received initial metadata */ bool has_initial_md_been_received; @@ -458,7 +458,7 @@ static void destroy_call(grpc_exec_ctx *exec_ctx, void *call, for (i = 0; i < STATUS_SOURCE_COUNT; i++) { GRPC_ERROR_UNREF( - unpack_received_status(gpr_atm_no_barrier_load(&c->status[i])).error); + unpack_received_status(gpr_atm_acq_load(&c->status[i])).error); } grpc_call_stack_destroy(exec_ctx, CALL_STACK_FROM_CALL(c), &c->final_info, c); @@ -490,7 +490,7 @@ void grpc_call_destroy(grpc_call *c) { GPR_ASSERT(!c->destroy_called); c->destroy_called = 1; - cancel = gpr_atm_no_barrier_load(&c->num_ops_sent) && !c->received_final_op; + cancel = gpr_atm_acq_load(&c->any_ops_sent_atm) && !gpr_atm_acq_load(&c->received_final_op_atm); if (cancel) { cancel_with_error(&exec_ctx, c, STATUS_FROM_API_OVERRIDE, GRPC_ERROR_CANCELLED); @@ -1048,7 +1048,7 @@ static void finish_batch_completion(grpc_exec_ctx *exec_ctx, void *user_data, } static grpc_error *consolidate_batch_errors(batch_control *bctl) { - size_t n = (size_t)gpr_atm_no_barrier_load(&bctl->num_errors); + size_t n = (size_t)gpr_atm_acq_load(&bctl->num_errors); if (n == 0) { return GRPC_ERROR_NONE; } else if (n == 1) { @@ -1093,7 +1093,7 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, &call->metadata_batch[1 /* is_receiving */][1 /* is_trailing */]; recv_trailing_filter(exec_ctx, call, md); - call->received_final_op = true; + gpr_atm_rel_store(&call->received_final_op_atm, 1); /* propagate cancellation to any interested children */ if (gpr_atm_acq_load(&call->has_children)) { gpr_mu_lock(&call->child_list_mu); @@ -1286,7 +1286,7 @@ static void validate_filtered_metadata(grpc_exec_ctx *exec_ctx, static void add_batch_error(grpc_exec_ctx *exec_ctx, batch_control *bctl, grpc_error *error, bool has_cancelled) { if (error == GRPC_ERROR_NONE) return; - int idx = (int)gpr_atm_no_barrier_fetch_add(&bctl->num_errors, 1); + int idx = (int)gpr_atm_full_fetch_add(&bctl->num_errors, 1); if (idx == 0 && !has_cancelled) { cancel_with_error(exec_ctx, bctl->call, STATUS_FROM_CORE, GRPC_ERROR_REF(error)); @@ -1664,7 +1664,7 @@ static grpc_call_error call_start_batch(grpc_exec_ctx *exec_ctx, grpc_closure_init(&bctl->finish_batch, finish_batch, bctl, grpc_schedule_on_exec_ctx); stream_op->on_complete = &bctl->finish_batch; - gpr_atm_no_barrier_fetch_add(&call->num_ops_sent, 1); + gpr_atm_rel_store(&call->any_ops_sent_atm, 1); execute_op(exec_ctx, call, stream_op); From 123c72bc3c7594e31620e603dce4322c4d7c57b1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 10 Mar 2017 07:33:27 -0800 Subject: [PATCH 07/14] Handle a race between cancellation of a parent call and adding a child --- src/core/lib/surface/call.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index eafeef0a207..01efcafceba 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -163,7 +163,7 @@ struct grpc_call { bool requested_final_op; gpr_atm any_ops_sent_atm; gpr_atm received_final_op_atm; - + /* have we received initial metadata */ bool has_initial_md_been_received; @@ -343,6 +343,10 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, } if (args->propagation_mask & GRPC_PROPAGATE_CANCELLATION) { call->cancellation_is_inherited = 1; + if (gpr_atm_acq_load(&args->parent_call->received_final_op_atm)) { + cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE, + GRPC_ERROR_CANCELLED); + } } if (args->parent_call->first_child == NULL) { @@ -490,7 +494,8 @@ void grpc_call_destroy(grpc_call *c) { GPR_ASSERT(!c->destroy_called); c->destroy_called = 1; - cancel = gpr_atm_acq_load(&c->any_ops_sent_atm) && !gpr_atm_acq_load(&c->received_final_op_atm); + cancel = gpr_atm_acq_load(&c->any_ops_sent_atm) && + !gpr_atm_acq_load(&c->received_final_op_atm); if (cancel) { cancel_with_error(&exec_ctx, c, STATUS_FROM_API_OVERRIDE, GRPC_ERROR_CANCELLED); @@ -1103,7 +1108,8 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, next_child_call = child_call->sibling_next; if (child_call->cancellation_is_inherited) { GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); - grpc_call_cancel(child_call, NULL); + cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE, + GRPC_ERROR_CANCELLED); GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); } child_call = next_child_call; From c5b90df9b2738076acf6a6972d833489f6470335 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 10 Mar 2017 16:11:08 -0800 Subject: [PATCH 08/14] debug --- src/core/lib/surface/call.c | 74 +++++++++--------------------- src/core/lib/transport/transport.c | 3 +- 2 files changed, 24 insertions(+), 53 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 01efcafceba..8b2f9fd3f53 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -143,9 +143,8 @@ struct grpc_call { grpc_channel *channel; grpc_call *parent; grpc_call *first_child; - gpr_atm has_children; gpr_timespec start_time; - /* protects first_child, setting has_children, and child next/prev links */ + /* protects first_child, and child next/prev links */ gpr_mu child_list_mu; /* client or server call */ @@ -315,7 +314,6 @@ grpc_error *grpc_call_create(grpc_exec_ctx *exec_ctx, GPR_ASSERT(!args->parent_call->is_client); gpr_mu_lock(&args->parent_call->child_list_mu); - gpr_atm_rel_store(&args->parent_call->has_children, 1); if (args->propagation_mask & GRPC_PROPAGATE_DEADLINE) { send_deadline = gpr_time_min( @@ -566,45 +564,19 @@ grpc_call_error grpc_call_cancel_with_status(grpc_call *c, return GRPC_CALL_OK; } -typedef struct termination_closure { - grpc_closure closure; - grpc_call *call; - grpc_transport_stream_op op; -} termination_closure; - -static void done_termination(grpc_exec_ctx *exec_ctx, void *tcp, - grpc_error *error) { - termination_closure *tc = tcp; - GRPC_CALL_INTERNAL_UNREF(exec_ctx, tc->call, "termination"); - gpr_free(tc); -} - -static void send_termination(grpc_exec_ctx *exec_ctx, void *tcp, +static void done_termination(grpc_exec_ctx *exec_ctx, void *call, grpc_error *error) { - termination_closure *tc = tcp; - memset(&tc->op, 0, sizeof(tc->op)); - tc->op.cancel_error = GRPC_ERROR_REF(error); - /* reuse closure to catch completion */ - tc->op.on_complete = grpc_closure_init(&tc->closure, done_termination, tc, - grpc_schedule_on_exec_ctx); - execute_op(exec_ctx, tc->call, &tc->op); -} - -static void terminate_with_error(grpc_exec_ctx *exec_ctx, grpc_call *c, - grpc_error *error) { - termination_closure *tc = gpr_malloc(sizeof(*tc)); - memset(tc, 0, sizeof(*tc)); - tc->call = c; - GRPC_CALL_INTERNAL_REF(tc->call, "termination"); - grpc_closure_sched(exec_ctx, grpc_closure_init(&tc->closure, send_termination, - tc, grpc_schedule_on_exec_ctx), - error); + GRPC_CALL_INTERNAL_UNREF(exec_ctx, call, "termination"); } static void cancel_with_error(grpc_exec_ctx *exec_ctx, grpc_call *c, status_source source, grpc_error *error) { + GRPC_CALL_INTERNAL_REF(c, "termination"); set_status_from_error(exec_ctx, c, source, GRPC_ERROR_REF(error)); - terminate_with_error(exec_ctx, c, error); + grpc_transport_stream_op *op = grpc_make_transport_stream_op( + grpc_closure_create(done_termination, c, grpc_schedule_on_exec_ctx)); + op->cancel_error = error; + execute_op(exec_ctx, c, op); } static grpc_error *error_from_status(grpc_status_code status, @@ -1100,23 +1072,21 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, gpr_atm_rel_store(&call->received_final_op_atm, 1); /* propagate cancellation to any interested children */ - if (gpr_atm_acq_load(&call->has_children)) { - gpr_mu_lock(&call->child_list_mu); - child_call = call->first_child; - if (child_call != NULL) { - do { - next_child_call = child_call->sibling_next; - if (child_call->cancellation_is_inherited) { - GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); - cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE, - GRPC_ERROR_CANCELLED); - GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); - } - child_call = next_child_call; - } while (child_call != call->first_child); - } - gpr_mu_unlock(&call->child_list_mu); + gpr_mu_lock(&call->child_list_mu); + child_call = call->first_child; + if (child_call != NULL) { + do { + next_child_call = child_call->sibling_next; + if (child_call->cancellation_is_inherited) { + GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); + cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE, + GRPC_ERROR_CANCELLED); + GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); + } + child_call = next_child_call; + } while (child_call != call->first_child); } + gpr_mu_unlock(&call->child_list_mu); if (call->is_client) { get_final_status(call, set_status_value_directly, diff --git a/src/core/lib/transport/transport.c b/src/core/lib/transport/transport.c index 165950e288e..3024f2ae781 100644 --- a/src/core/lib/transport/transport.c +++ b/src/core/lib/transport/transport.c @@ -254,8 +254,9 @@ typedef struct { static void destroy_made_transport_stream_op(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { made_transport_stream_op *op = arg; - grpc_closure_sched(exec_ctx, op->inner_on_complete, GRPC_ERROR_REF(error)); + grpc_closure *c = op->inner_on_complete; gpr_free(op); + grpc_closure_run(exec_ctx, c, GRPC_ERROR_REF(error)); } grpc_transport_stream_op *grpc_make_transport_stream_op( From b18c8ba0b6bbc2317dd92353288f2b1c0ff353d5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 13 Mar 2017 15:51:37 -0700 Subject: [PATCH 09/14] Fix cancellation --- src/core/lib/surface/call.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 8b2f9fd3f53..47f36be5fda 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -1070,8 +1070,8 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, &call->metadata_batch[1 /* is_receiving */][1 /* is_trailing */]; recv_trailing_filter(exec_ctx, call, md); - gpr_atm_rel_store(&call->received_final_op_atm, 1); /* propagate cancellation to any interested children */ + gpr_atm_rel_store(&call->received_final_op_atm, 1); gpr_mu_lock(&call->child_list_mu); child_call = call->first_child; if (child_call != NULL) { @@ -1079,7 +1079,7 @@ static void post_batch_completion(grpc_exec_ctx *exec_ctx, next_child_call = child_call->sibling_next; if (child_call->cancellation_is_inherited) { GRPC_CALL_INTERNAL_REF(child_call, "propagate_cancel"); - cancel_with_error(exec_ctx, call, STATUS_FROM_API_OVERRIDE, + cancel_with_error(exec_ctx, child_call, STATUS_FROM_API_OVERRIDE, GRPC_ERROR_CANCELLED); GRPC_CALL_INTERNAL_UNREF(exec_ctx, child_call, "propagate_cancel"); } From dd9b597bd7ba13b135bf0e5ae292a9ad38d7d856 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Mar 2017 07:39:45 -0700 Subject: [PATCH 10/14] UBSAN cleanup --- Makefile | 2 +- build.yaml | 6 +++--- tools/run_tests/generated/configs.json | 2 +- tools/ubsan_suppressions.txt | 7 +++++++ 4 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 tools/ubsan_suppressions.txt diff --git a/Makefile b/Makefile index 57e877beb01..addcc914b13 100644 --- a/Makefile +++ b/Makefile @@ -189,7 +189,7 @@ CC_ubsan = clang CXX_ubsan = clang++ LD_ubsan = clang LDXX_ubsan = clang++ -CPPFLAGS_ubsan = -O0 -fsanitize-coverage=edge -fsanitize=undefined,unsigned-integer-overflow -fno-omit-frame-pointer -Wno-unused-command-line-argument -Wvarargs +CPPFLAGS_ubsan = -O0 -fsanitize-coverage=edge -fsanitize=undefined -fno-omit-frame-pointer -Wno-unused-command-line-argument -Wvarargs LDFLAGS_ubsan = -fsanitize=undefined,unsigned-integer-overflow DEFINES_ubsan = NDEBUG diff --git a/build.yaml b/build.yaml index 9ff37d59e17..321a8ac381d 100644 --- a/build.yaml +++ b/build.yaml @@ -4090,8 +4090,8 @@ configs: TSAN_OPTIONS: suppressions=tools/tsan_suppressions.txt:halt_on_error=1:second_deadlock_stack=1 ubsan: CC: clang - CPPFLAGS: -O0 -fsanitize-coverage=edge -fsanitize=undefined,unsigned-integer-overflow - -fno-omit-frame-pointer -Wno-unused-command-line-argument -Wvarargs + CPPFLAGS: -O0 -fsanitize-coverage=edge -fsanitize=undefined -fno-omit-frame-pointer + -Wno-unused-command-line-argument -Wvarargs CXX: clang++ DEFINES: NDEBUG LD: clang @@ -4099,7 +4099,7 @@ configs: LDXX: clang++ compile_the_world: true test_environ: - UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1 + UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1:suppressions=tools/ubsan_suppressions.txt defaults: benchmark: CPPFLAGS: -Ithird_party/benchmark/include -DHAVE_POSIX_REGEX diff --git a/tools/run_tests/generated/configs.json b/tools/run_tests/generated/configs.json index 9173bd7c19c..69e0f447743 100644 --- a/tools/run_tests/generated/configs.json +++ b/tools/run_tests/generated/configs.json @@ -58,7 +58,7 @@ { "config": "ubsan", "environ": { - "UBSAN_OPTIONS": "halt_on_error=1:print_stacktrace=1" + "UBSAN_OPTIONS": "halt_on_error=1:print_stacktrace=1:suppressions=tools/ubsan_suppressions.txt" } }, { diff --git a/tools/ubsan_suppressions.txt b/tools/ubsan_suppressions.txt new file mode 100644 index 00000000000..9869f98a22e --- /dev/null +++ b/tools/ubsan_suppressions.txt @@ -0,0 +1,7 @@ +# boringssl stuff +nonnull-attribute:bn_wexpand +nonnull-attribute:CBB_add_bytes +nonnull-attribute:rsa_blinding_get +nonnull-attribute:ssl_copy_key_material +alignment:CRYPTO_cbc128_encrypt + From ff23f801c5fbb67d42b70c4ba0ba28ae2a7d31ae Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Mar 2017 07:50:56 -0700 Subject: [PATCH 11/14] Fix crash --- test/cpp/microbenchmarks/bm_arena.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/cpp/microbenchmarks/bm_arena.cc b/test/cpp/microbenchmarks/bm_arena.cc index 33e073f7495..770c0b6d474 100644 --- a/test/cpp/microbenchmarks/bm_arena.cc +++ b/test/cpp/microbenchmarks/bm_arena.cc @@ -48,8 +48,15 @@ BENCHMARK(BM_Arena_NoOp)->Range(1, 1024 * 1024); static void BM_Arena_ManyAlloc(benchmark::State& state) { gpr_arena* a = gpr_arena_create(state.range(0)); + const size_t realloc_after = + 1024 * 1024 * 1024 / ((state.range(1) + 15) & 0xffffff0u); while (state.KeepRunning()) { gpr_arena_alloc(a, state.range(1)); + // periodically recreate arena to avoid OOM + if (state.iterations() % realloc_after == 0) { + gpr_arena_destroy(a); + a = gpr_arena_create(state.range(0)); + } } gpr_arena_destroy(a); } From d26250be35a0e0921cb97f43b13ce14343ee4c7d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 14 Mar 2017 11:01:52 -0700 Subject: [PATCH 12/14] Get growth right --- src/core/lib/support/arena.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/lib/support/arena.c b/src/core/lib/support/arena.c index 6610acd3a07..7bcb983f245 100644 --- a/src/core/lib/support/arena.c +++ b/src/core/lib/support/arena.c @@ -78,8 +78,7 @@ void *gpr_arena_alloc(gpr_arena *arena, size_t size) { while (start > z->size_end) { zone *next_z = (zone *)gpr_atm_acq_load(&z->next_atm); if (next_z == NULL) { - size_t next_z_size = - GPR_MAX((size_t)gpr_atm_no_barrier_load(&arena->size_so_far), size); + size_t next_z_size = (size_t)gpr_atm_no_barrier_load(&arena->size_so_far); next_z = gpr_zalloc(sizeof(zone) + next_z_size); next_z->size_begin = z->size_end; next_z->size_end = z->size_end + next_z_size; From 39e2e09f20dcd55339a01cef4b7b95b2d6ff30c9 Mon Sep 17 00:00:00 2001 From: Yong Ni Date: Tue, 14 Mar 2017 13:50:57 -0700 Subject: [PATCH 13/14] created gcr upload script for version compatiblity tests. --- tools/gcp/utils/gcr_upload.py | 119 ++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100755 tools/gcp/utils/gcr_upload.py diff --git a/tools/gcp/utils/gcr_upload.py b/tools/gcp/utils/gcr_upload.py new file mode 100755 index 00000000000..b22f8731f68 --- /dev/null +++ b/tools/gcp/utils/gcr_upload.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python2.7 +# Copyright 2017, 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. + +"""Upload docker images to Google Container Registry.""" + +from __future__ import print_function + +import argparse +import atexit +import os +import shutil +import subprocess +import tempfile + +argp = argparse.ArgumentParser(description='Run interop tests.') +argp.add_argument('--gcr_path', + default='gcr.io/grpc-testing', + help='Path of docker images in Google Container Registry') + +argp.add_argument('--gcr_tag', + default='latest', + help='the tag string for the images to upload') + +argp.add_argument('--with_files', + default=[], + nargs='+', + help='additional files to include in the docker image') + +argp.add_argument('--with_file_dest', + default='/var/local/image_info', + help='Destination directory for with_files inside docker image') + +argp.add_argument('--images', + default=[], + nargs='+', + help='local docker images in the form of repo:tag ' + + '(i.e. grpc_interop_java:26328ad8) to upload') + +argp.add_argument('--keep', + action='store_true', + help='keep the created local images after uploading to GCR') + + +args = argp.parse_args() + +def upload_to_gcr(image): + """Tags and Pushes a docker image in Google Containger Registry. + + image: docker image name, i.e. grpc_interop_java:26328ad8 + + A docker image image_foo:tag_old will be uploaded as + /image_foo: + after inserting extra with_files under with_file_dest in the image. The + original image name will be stored as label original_name:"image_foo:tag_old". + """ + tag_idx = image.find(':') + if tag_idx == -1: + print('Failed to parse docker image name %s' % image) + return False + new_tag = '%s/%s:%s' % (args.gcr_path, image[:tag_idx], args.gcr_tag) + + lines = ['FROM ' + image] + lines.append('LABEL original_name="%s"' % image) + + temp_dir = tempfile.mkdtemp() + atexit.register(lambda: subprocess.call(['rm', '-rf', temp_dir])) + + # Copy with_files inside the tmp directory, which will be the docker build + # context. + for f in args.with_files: + shutil.copy(f, temp_dir) + lines.append('COPY %s %s/' % (os.path.basename(f), args.with_file_dest)) + + # Create a Dockerfile. + with open(os.path.join(temp_dir, 'Dockerfile'), 'w') as f: + f.write('\n'.join(lines)) + + build_cmd = ['docker', 'build', '--rm', '--tag', new_tag, temp_dir] + subprocess.check_output(build_cmd) + + if not args.keep: + atexit.register(lambda: subprocess.call(['docker', 'rmi', new_tag])) + + # Upload to GCR. + if args.gcr_path: + subprocess.call(['gcloud', 'docker', '--', 'push', new_tag]) + + return True + + +for image in args.images: + upload_to_gcr(image) From 488d41912567708ad4934ad35b41ac7275d65ec9 Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Wed, 15 Mar 2017 23:09:52 +0000 Subject: [PATCH 14/14] Update health to current gRPC code elements --- src/python/grpcio_health_checking/grpc_health/v1/health.py | 3 ++- .../grpcio_tests/tests/health_check/_health_servicer_test.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/python/grpcio_health_checking/grpc_health/v1/health.py b/src/python/grpcio_health_checking/grpc_health/v1/health.py index f0f11cf84b0..e92c2659b7e 100644 --- a/src/python/grpcio_health_checking/grpc_health/v1/health.py +++ b/src/python/grpcio_health_checking/grpc_health/v1/health.py @@ -33,9 +33,10 @@ import threading import grpc from grpc_health.v1 import health_pb2 +from grpc_health.v1 import health_pb2_grpc -class HealthServicer(health_pb2.HealthServicer): +class HealthServicer(health_pb2_grpc.HealthServicer): """Servicer handling RPCs for service statuses.""" def __init__(self): diff --git a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py index 363b4c5f994..1bc8669dad9 100644 --- a/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py +++ b/src/python/grpcio_tests/tests/health_check/_health_servicer_test.py @@ -34,6 +34,7 @@ import grpc from grpc.framework.foundation import logging_pool from grpc_health.v1 import health from grpc_health.v1 import health_pb2 +from grpc_health.v1 import health_pb2_grpc from tests.unit.framework.common import test_constants @@ -52,11 +53,11 @@ class HealthServicerTest(unittest.TestCase): server_pool = logging_pool.pool(test_constants.THREAD_CONCURRENCY) self._server = grpc.server(server_pool) port = self._server.add_insecure_port('[::]:0') - health_pb2.add_HealthServicer_to_server(servicer, self._server) + health_pb2_grpc.add_HealthServicer_to_server(servicer, self._server) self._server.start() channel = grpc.insecure_channel('localhost:%d' % port) - self._stub = health_pb2.HealthStub(channel) + self._stub = health_pb2_grpc.HealthStub(channel) def test_empty_service(self): request = health_pb2.HealthCheckRequest()