From 8f2a054f1706fcda1d821b4c9dea250d0cb47d29 Mon Sep 17 00:00:00 2001 From: Michael Bausor Date: Wed, 12 Jul 2017 09:54:26 -0700 Subject: [PATCH 01/26] Support parameter to change PHP generated stub suffix --- src/compiler/php_generator.cc | 12 +++++++----- src/compiler/php_generator.h | 3 ++- src/compiler/php_generator_helpers.h | 17 +++++++++++++++-- src/compiler/php_plugin.cc | 5 +++-- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/compiler/php_generator.cc b/src/compiler/php_generator.cc index 6d34761fdfd..38ec46e656e 100644 --- a/src/compiler/php_generator.cc +++ b/src/compiler/php_generator.cc @@ -97,13 +97,14 @@ void PrintMethod(const MethodDescriptor *method, Printer *out) { } // Prints out the service descriptor object -void PrintService(const ServiceDescriptor *service, Printer *out) { +void PrintService(const ServiceDescriptor *service, + const grpc::string ¶meter, Printer *out) { map vars; out->Print("/**\n"); out->Print(GetPHPComments(service, " *").c_str()); out->Print(" */\n"); - vars["name"] = service->name(); - out->Print(vars, "class $name$Client extends \\Grpc\\BaseStub {\n\n"); + vars["name"] = GetPHPServiceClassname(service, parameter); + out->Print(vars, "class $name$ extends \\Grpc\\BaseStub {\n\n"); out->Indent(); out->Indent(); out->Print( @@ -131,7 +132,8 @@ void PrintService(const ServiceDescriptor *service, Printer *out) { } grpc::string GenerateFile(const FileDescriptor *file, - const ServiceDescriptor *service) { + const ServiceDescriptor *service, + const grpc::string ¶meter) { grpc::string output; { StringOutputStream output_stream(&output); @@ -150,7 +152,7 @@ grpc::string GenerateFile(const FileDescriptor *file, vars["package"] = MessageIdentifierName(file->package()); out.Print(vars, "namespace $package$;\n\n"); - PrintService(service, &out); + PrintService(service, parameter, &out); } return output; } diff --git a/src/compiler/php_generator.h b/src/compiler/php_generator.h index 4518bc24f91..9a04bd33d70 100644 --- a/src/compiler/php_generator.h +++ b/src/compiler/php_generator.h @@ -24,7 +24,8 @@ namespace grpc_php_generator { grpc::string GenerateFile(const grpc::protobuf::FileDescriptor *file, - const grpc::protobuf::ServiceDescriptor *service); + const grpc::protobuf::ServiceDescriptor *service, + const grpc::string ¶meter); } // namespace grpc_php_generator diff --git a/src/compiler/php_generator_helpers.h b/src/compiler/php_generator_helpers.h index 3a5c08b3e69..5edebf6290a 100644 --- a/src/compiler/php_generator_helpers.h +++ b/src/compiler/php_generator_helpers.h @@ -26,9 +26,22 @@ namespace grpc_php_generator { +inline grpc::string GetPHPServiceClassname( + const grpc::protobuf::ServiceDescriptor *service, + const grpc::string ¶meter) { + grpc::string suffix; + if (parameter == "") { + suffix = "Client"; + } else { + suffix = parameter; + } + return service->name() + suffix; +} + inline grpc::string GetPHPServiceFilename( const grpc::protobuf::FileDescriptor *file, - const grpc::protobuf::ServiceDescriptor *service) { + const grpc::protobuf::ServiceDescriptor *service, + const grpc::string ¶meter) { std::vector tokens = grpc_generator::tokenize(file->package(), "."); std::ostringstream oss; @@ -36,7 +49,7 @@ inline grpc::string GetPHPServiceFilename( oss << (i == 0 ? "" : "/") << grpc_generator::CapitalizeFirstLetter(tokens[i]); } - return oss.str() + "/" + service->name() + "Client.php"; + return oss.str() + "/" + GetPHPServiceClassname(service, parameter) + ".php"; } // ReplaceAll replaces all instances of search with replace in s. diff --git a/src/compiler/php_plugin.cc b/src/compiler/php_plugin.cc index 7a581fd7bcb..bbe91656d5e 100644 --- a/src/compiler/php_plugin.cc +++ b/src/compiler/php_plugin.cc @@ -41,10 +41,11 @@ class PHPGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { } for (int i = 0; i < file->service_count(); i++) { - grpc::string code = GenerateFile(file, file->service(i)); + grpc::string code = GenerateFile(file, file->service(i), parameter); // Get output file name - grpc::string file_name = GetPHPServiceFilename(file, file->service(i)); + grpc::string file_name = + GetPHPServiceFilename(file, file->service(i), parameter); std::unique_ptr output( context->Open(file_name)); From abe3cf51f02d48b28fd67d78e7a3060d763148b5 Mon Sep 17 00:00:00 2001 From: Guantao Liu Date: Wed, 12 Jul 2017 17:36:30 -0700 Subject: [PATCH 02/26] Add a new metric 'Queries/CPU-sec'. Enable internal credential types. --- src/proto/grpc/testing/control.proto | 5 ++++ test/cpp/qps/BUILD | 2 ++ test/cpp/qps/client.h | 14 ++++++++-- test/cpp/qps/driver.cc | 36 ++++++++++++++++++++----- test/cpp/qps/driver.h | 5 ++-- test/cpp/qps/qps_json_driver.cc | 12 ++++++--- test/cpp/qps/qps_worker.cc | 8 ++++-- test/cpp/qps/qps_worker.h | 3 ++- test/cpp/qps/report.cc | 21 +++++++++++++++ test/cpp/qps/report.h | 7 +++++ test/cpp/qps/server.h | 15 ++++++----- test/cpp/qps/worker.cc | 5 +++- test/cpp/util/create_test_channel.cc | 40 +++++++++++++++++++++------- test/cpp/util/create_test_channel.h | 6 +++++ 14 files changed, 146 insertions(+), 33 deletions(-) diff --git a/src/proto/grpc/testing/control.proto b/src/proto/grpc/testing/control.proto index 48016642f74..2ff2e4e8a28 100644 --- a/src/proto/grpc/testing/control.proto +++ b/src/proto/grpc/testing/control.proto @@ -64,6 +64,7 @@ message LoadParams { message SecurityParams { bool use_test_ca = 1; string server_host_override = 2; + string cred_type = 3; } message ChannelArg { @@ -240,6 +241,10 @@ message ScenarioResultSummary // Number of polls called inside completion queue per request double client_polls_per_request = 15; double server_polls_per_request = 16; + + // Queries per CPU-sec over all servers or clients + double server_queries_per_cpu_sec = 17; + double client_queries_per_cpu_sec = 18; } // Results of a single benchmark scenario. diff --git a/test/cpp/qps/BUILD b/test/cpp/qps/BUILD index 4d8b3b40e3c..a3f70cf8b5b 100644 --- a/test/cpp/qps/BUILD +++ b/test/cpp/qps/BUILD @@ -77,6 +77,7 @@ grpc_cc_library( "//src/proto/grpc/testing:services_proto", "//test/core/util:gpr_test_util", "//test/core/util:grpc_test_util", + "//test/cpp/util:test_util", ], ) @@ -141,6 +142,7 @@ grpc_cc_binary( ":benchmark_config", ":driver_impl", "//:grpc++", + "//test/cpp/util:test_config", ], external_deps = [ "gflags", diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index e759446c1ae..f47f378a984 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -39,6 +39,7 @@ #include "test/cpp/qps/interarrival.h" #include "test/cpp/qps/usage_timer.h" #include "test/cpp/util/create_test_channel.h" +#include "test/cpp/util/test_credentials_provider.h" namespace grpc { namespace testing { @@ -407,9 +408,18 @@ class ClientImpl : public Client { ChannelArguments args; args.SetInt("shard_to_ensure_no_subchannel_merges", shard); set_channel_args(config, &args); + + grpc::string type; + if (config.has_security_params() && + config.security_params().cred_type().empty()) { + type = kTlsCredentialsType; + } else { + type = config.security_params().cred_type(); + } + channel_ = CreateTestChannel( - target, config.security_params().server_host_override(), - config.has_security_params(), !config.security_params().use_test_ca(), + target, type, config.security_params().server_host_override(), + !config.security_params().use_test_ca(), std::shared_ptr(), args); gpr_log(GPR_INFO, "Connecting to %s", target.c_str()); GPR_ASSERT(channel_->WaitForConnected( diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index fbd8d1b1e76..bb4e7536e84 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -40,6 +40,7 @@ #include "test/cpp/qps/histogram.h" #include "test/cpp/qps/qps_worker.h" #include "test/cpp/qps/stats.h" +#include "test/cpp/util/test_credentials_provider.h" using std::list; using std::thread; @@ -172,13 +173,27 @@ static void postprocess_scenario_result(ScenarioResult* result) { sum(result->client_stats(), CliPollCount) / histogram.Count()); result->mutable_summary()->set_server_polls_per_request( sum(result->server_stats(), SvrPollCount) / histogram.Count()); + + auto server_queries_per_cpu_sec = + histogram.Count() / + (sum(result->server_stats(), ServerSystemTime) + + sum(result->server_stats(), ServerUserTime)); + auto client_queries_per_cpu_sec = + histogram.Count() / + (sum(result->client_stats(), SystemTime) + + sum(result->client_stats(), UserTime)); + + result->mutable_summary()->set_server_queries_per_cpu_sec( + server_queries_per_cpu_sec); + result->mutable_summary()->set_client_queries_per_cpu_sec( + client_queries_per_cpu_sec); } std::unique_ptr RunScenario( const ClientConfig& initial_client_config, size_t num_clients, const ServerConfig& initial_server_config, size_t num_servers, int warmup_seconds, int benchmark_seconds, int spawn_local_worker_count, - const char* qps_server_target_override) { + const char* qps_server_target_override, const char* credential_type) { // Log everything from the driver gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); @@ -214,7 +229,7 @@ std::unique_ptr RunScenario( } int driver_port = grpc_pick_unused_port_or_die(); - local_workers.emplace_back(new QpsWorker(driver_port)); + local_workers.emplace_back(new QpsWorker(driver_port, 0, credential_type)); char addr[256]; sprintf(addr, "localhost:%d", driver_port); if (spawn_local_worker_count < 0) { @@ -246,12 +261,15 @@ std::unique_ptr RunScenario( }; std::vector servers(num_servers); std::unordered_map> hosts_cores; + ChannelArguments channel_args; for (size_t i = 0; i < num_servers; i++) { gpr_log(GPR_INFO, "Starting server on %s (worker #%" PRIuPTR ")", workers[i].c_str(), i); servers[i].stub = WorkerService::NewStub( - CreateChannel(workers[i], InsecureChannelCredentials())); + CreateChannel(workers[i], + GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); ServerConfig server_config = initial_server_config; if (server_config.core_limit() != 0) { @@ -298,7 +316,9 @@ std::unique_ptr RunScenario( gpr_log(GPR_INFO, "Starting client on %s (worker #%" PRIuPTR ")", worker.c_str(), i + num_servers); clients[i].stub = WorkerService::NewStub( - CreateChannel(worker, InsecureChannelCredentials())); + CreateChannel(worker, + GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); ClientConfig per_client_config = client_config; if (initial_client_config.core_limit() != 0) { @@ -483,16 +503,20 @@ std::unique_ptr RunScenario( return result; } -bool RunQuit() { +bool RunQuit(const char* credential_type) { // Get client, server lists bool result = true; auto workers = get_workers("QPS_WORKERS"); if (workers.size() == 0) { return false; } + + ChannelArguments channel_args; for (size_t i = 0; i < workers.size(); i++) { auto stub = WorkerService::NewStub( - CreateChannel(workers[i], InsecureChannelCredentials())); + CreateChannel(workers[i], + GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); Void dummy; grpc::ClientContext ctx; ctx.set_wait_for_ready(true); diff --git a/test/cpp/qps/driver.h b/test/cpp/qps/driver.h index def32c6f0ef..ddf75ce5872 100644 --- a/test/cpp/qps/driver.h +++ b/test/cpp/qps/driver.h @@ -31,9 +31,10 @@ std::unique_ptr RunScenario( const grpc::testing::ClientConfig& client_config, size_t num_clients, const grpc::testing::ServerConfig& server_config, size_t num_servers, int warmup_seconds, int benchmark_seconds, int spawn_local_worker_count, - const char* qps_server_target_override = ""); + const char* qps_server_target_override = "", + const char* credential_type = "INSECURE_CREDENTIALS"); -bool RunQuit(); +bool RunQuit(const char* credential_type = "INSECURE_CREDENTIALS"); } // namespace testing } // namespace grpc diff --git a/test/cpp/qps/qps_json_driver.cc b/test/cpp/qps/qps_json_driver.cc index 590c22ec29f..d4698a7dc2f 100644 --- a/test/cpp/qps/qps_json_driver.cc +++ b/test/cpp/qps/qps_json_driver.cc @@ -30,6 +30,7 @@ #include "test/cpp/qps/driver.h" #include "test/cpp/qps/parse_json.h" #include "test/cpp/qps/report.h" +#include "test/cpp/util/test_config.h" DEFINE_string(scenarios_file, "", "JSON file containing an array of Scenario objects"); @@ -60,6 +61,9 @@ DEFINE_string(qps_server_target_override, "", DEFINE_string(json_file_out, "", "File to write the JSON output to."); +DEFINE_string(credential_type, "INSECURE_CREDENTIALS", + "Credential type for communication with workers"); + namespace grpc { namespace testing { @@ -71,7 +75,8 @@ static std::unique_ptr RunAndReport(const Scenario& scenario, scenario.server_config(), scenario.num_servers(), scenario.warmup_seconds(), scenario.benchmark_seconds(), scenario.spawn_local_worker_count(), - FLAGS_qps_server_target_override.c_str()); + FLAGS_qps_server_target_override.c_str(), + FLAGS_credential_type.c_str()); // Amend the result with scenario config. Eventually we should adjust // RunScenario contract so we don't need to touch the result here. @@ -83,6 +88,7 @@ static std::unique_ptr RunAndReport(const Scenario& scenario, GetReporter()->ReportTimes(*result); GetReporter()->ReportCpuUsage(*result); GetReporter()->ReportPollCount(*result); + GetReporter()->ReportQueriesPerCpuSec(*result); for (int i = 0; *success && i < result->client_success_size(); i++) { *success = result->client_success(i); @@ -184,7 +190,7 @@ static bool QpsDriver() { } else if (scjson) { json = FLAGS_scenarios_json.c_str(); } else if (FLAGS_quit) { - return RunQuit(); + return RunQuit(FLAGS_credential_type.c_str()); } // Parse into an array of scenarios @@ -219,7 +225,7 @@ static bool QpsDriver() { } // namespace grpc int main(int argc, char** argv) { - grpc::testing::InitBenchmark(&argc, &argv, true); + grpc::testing::InitTest(&argc, &argv, true); bool ok = grpc::testing::QpsDriver(); diff --git a/test/cpp/qps/qps_worker.cc b/test/cpp/qps/qps_worker.cc index 10bc5422e13..4db4c868241 100644 --- a/test/cpp/qps/qps_worker.cc +++ b/test/cpp/qps/qps_worker.cc @@ -41,6 +41,7 @@ #include "test/cpp/qps/client.h" #include "test/cpp/qps/server.h" #include "test/cpp/util/create_test_channel.h" +#include "test/cpp/util/test_credentials_provider.h" namespace grpc { namespace testing { @@ -263,7 +264,8 @@ class WorkerServiceImpl final : public WorkerService::Service { QpsWorker* worker_; }; -QpsWorker::QpsWorker(int driver_port, int server_port) { +QpsWorker::QpsWorker(int driver_port, int server_port, + const char* credential_type) { impl_.reset(new WorkerServiceImpl(server_port, this)); gpr_atm_rel_store(&done_, static_cast(0)); @@ -271,7 +273,9 @@ QpsWorker::QpsWorker(int driver_port, int server_port) { gpr_join_host_port(&server_address, "::", driver_port); ServerBuilder builder; - builder.AddListeningPort(server_address, InsecureServerCredentials()); + builder.AddListeningPort(server_address, + GetCredentialsProvider()->GetServerCredentials( + credential_type)); builder.RegisterService(impl_.get()); gpr_free(server_address); diff --git a/test/cpp/qps/qps_worker.h b/test/cpp/qps/qps_worker.h index c8a7be93605..ab5e26a10e4 100644 --- a/test/cpp/qps/qps_worker.h +++ b/test/cpp/qps/qps_worker.h @@ -33,7 +33,8 @@ class WorkerServiceImpl; class QpsWorker { public: - explicit QpsWorker(int driver_port, int server_port = 0); + explicit QpsWorker(int driver_port, int server_port = 0, + const char* credential_type = "INSECURE_CREDENTIALS"); ~QpsWorker(); bool Done() const; diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc index 809c5630442..a45b10bcb81 100644 --- a/test/cpp/qps/report.cc +++ b/test/cpp/qps/report.cc @@ -71,6 +71,12 @@ void CompositeReporter::ReportPollCount(const ScenarioResult& result) { } } +void CompositeReporter::ReportQueriesPerCpuSec(const ScenarioResult& result) { + for (size_t i = 0; i < reporters_.size(); ++i) { + reporters_[i]->ReportQueriesPerCpuSec(result); + } +} + void GprLogReporter::ReportQPS(const ScenarioResult& result) { gpr_log(GPR_INFO, "QPS: %.1f", result.summary().qps()); if (result.summary().failed_requests_per_second() > 0) { @@ -119,6 +125,13 @@ void GprLogReporter::ReportPollCount(const ScenarioResult& result) { result.summary().server_polls_per_request()); } +void GprLogReporter::ReportQueriesPerCpuSec(const ScenarioResult& result) { + gpr_log(GPR_INFO, "Server Queries/CPU-sec: %.2f", + result.summary().server_queries_per_cpu_sec()); + gpr_log(GPR_INFO, "Client Queries/CPU-sec: %.2f", + result.summary().client_queries_per_cpu_sec()); +} + void JsonReporter::ReportQPS(const ScenarioResult& result) { grpc::string json_string = SerializeJson(result, "type.googleapis.com/grpc.testing.ScenarioResult"); @@ -147,6 +160,10 @@ void JsonReporter::ReportPollCount(const ScenarioResult& result) { // NOP - all reporting is handled by ReportQPS. } +void JsonReporter::ReportQueriesPerCpuSec(const ScenarioResult& result) { + // NOP - all reporting is handled by ReportQPS. +} + void RpcReporter::ReportQPS(const ScenarioResult& result) { grpc::ClientContext context; grpc::Status status; @@ -183,5 +200,9 @@ void RpcReporter::ReportPollCount(const ScenarioResult& result) { // NOP - all reporting is handled by ReportQPS. } +void RpcReporter::ReportQueriesPerCpuSec(const ScenarioResult& result) { + // NOP - all reporting is handled by ReportQPS. +} + } // namespace testing } // namespace grpc diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index 0bd398fd2a2..321be2a97fa 100644 --- a/test/cpp/qps/report.h +++ b/test/cpp/qps/report.h @@ -64,6 +64,9 @@ class Reporter { /** Reports client and server poll usage inside completion queue. */ virtual void ReportPollCount(const ScenarioResult& result) = 0; + /** Reports queries per cpu-sec. */ + virtual void ReportQueriesPerCpuSec(const ScenarioResult& result) = 0; + private: const string name_; }; @@ -82,6 +85,7 @@ class CompositeReporter : public Reporter { void ReportTimes(const ScenarioResult& result) override; void ReportCpuUsage(const ScenarioResult& result) override; void ReportPollCount(const ScenarioResult& result) override; + void ReportQueriesPerCpuSec(const ScenarioResult& result) override; private: std::vector > reporters_; @@ -99,6 +103,7 @@ class GprLogReporter : public Reporter { void ReportTimes(const ScenarioResult& result) override; void ReportCpuUsage(const ScenarioResult& result) override; void ReportPollCount(const ScenarioResult& result) override; + void ReportQueriesPerCpuSec(const ScenarioResult& result) override; }; /** Dumps the report to a JSON file. */ @@ -114,6 +119,7 @@ class JsonReporter : public Reporter { void ReportTimes(const ScenarioResult& result) override; void ReportCpuUsage(const ScenarioResult& result) override; void ReportPollCount(const ScenarioResult& result) override; + void ReportQueriesPerCpuSec(const ScenarioResult& result) override; const string report_file_; }; @@ -130,6 +136,7 @@ class RpcReporter : public Reporter { void ReportTimes(const ScenarioResult& result) override; void ReportCpuUsage(const ScenarioResult& result) override; void ReportPollCount(const ScenarioResult& result) override; + void ReportQueriesPerCpuSec(const ScenarioResult& result) override; std::unique_ptr stub_; }; diff --git a/test/cpp/qps/server.h b/test/cpp/qps/server.h index 4b699e07080..c0dac96d8b8 100644 --- a/test/cpp/qps/server.h +++ b/test/cpp/qps/server.h @@ -32,6 +32,7 @@ #include "test/core/end2end/data/ssl_test_data.h" #include "test/core/util/port.h" #include "test/cpp/qps/usage_timer.h" +#include "test/cpp/util/test_credentials_provider.h" namespace grpc { namespace testing { @@ -89,12 +90,14 @@ class Server { static std::shared_ptr CreateServerCredentials( const ServerConfig& config) { if (config.has_security_params()) { - SslServerCredentialsOptions::PemKeyCertPair pkcp = {test_server1_key, - test_server1_cert}; - SslServerCredentialsOptions ssl_opts; - ssl_opts.pem_root_certs = ""; - ssl_opts.pem_key_cert_pairs.push_back(pkcp); - return SslServerCredentials(ssl_opts); + grpc::string type; + if (config.security_params().cred_type().empty()) { + type = kTlsCredentialsType; + } else { + type = config.security_params().cred_type(); + } + + return GetCredentialsProvider()->GetServerCredentials(type); } else { return InsecureServerCredentials(); } diff --git a/test/cpp/qps/worker.cc b/test/cpp/qps/worker.cc index fd51d32db0d..7a8036327f6 100644 --- a/test/cpp/qps/worker.cc +++ b/test/cpp/qps/worker.cc @@ -30,6 +30,8 @@ DEFINE_int32(driver_port, 0, "Port for communication with driver"); DEFINE_int32(server_port, 0, "Port for operation as a server"); +DEFINE_string(credential_type, "INSECURE_CREDENTIALS", + "Credential type for communication with driver"); static bool got_sigint = false; @@ -39,7 +41,8 @@ namespace grpc { namespace testing { static void RunServer() { - QpsWorker worker(FLAGS_driver_port, FLAGS_server_port); + QpsWorker worker(FLAGS_driver_port, FLAGS_server_port, + FLAGS_credential_type.c_str()); while (!got_sigint && !worker.Done()) { gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), diff --git a/test/cpp/util/create_test_channel.cc b/test/cpp/util/create_test_channel.cc index 68c6fe4622e..34b6d60d014 100644 --- a/test/cpp/util/create_test_channel.cc +++ b/test/cpp/util/create_test_channel.cc @@ -51,29 +51,31 @@ void AddProdSslType() { } // namespace -// When ssl is enabled, if server is empty, override_hostname is used to +// When cred_type is 'ssl', if server is empty, override_hostname is used to // create channel. Otherwise, connect to server and override hostname if // override_hostname is provided. -// When ssl is not enabled, override_hostname is ignored. +// When cred_type is not 'ssl', override_hostname is ignored. // Set use_prod_root to true to use the SSL root for connecting to google. // In this case, path to the roots pem file must be set via environment variable // GRPC_DEFAULT_SSL_ROOTS_FILE_PATH. // Otherwise, root for test SSL cert will be used. -// creds will be used to create a channel when enable_ssl is true. +// creds will be used to create a channel when cred_type is 'ssl'. // Use examples: // CreateTestChannel( -// "1.1.1.1:12345", "override.hostname.com", true, false, creds); -// CreateTestChannel("test.google.com:443", "", true, true, creds); +// "1.1.1.1:12345", "ssl", "override.hostname.com", false, creds); +// CreateTestChannel("test.google.com:443", "ssl", "", true, creds); // same as above -// CreateTestChannel("", "test.google.com:443", true, true, creds); +// CreateTestChannel("", "ssl", "test.google.com:443", true, creds); std::shared_ptr CreateTestChannel( - const grpc::string& server, const grpc::string& override_hostname, - bool enable_ssl, bool use_prod_roots, + const grpc::string& server, const grpc::string& cred_type, + const grpc::string& override_hostname, bool use_prod_roots, const std::shared_ptr& creds, const ChannelArguments& args) { ChannelArguments channel_args(args); std::shared_ptr channel_creds; - if (enable_ssl) { + if (cred_type.empty()) { + return CreateChannel(server, InsecureChannelCredentials()); + } else if (cred_type == testing::kTlsCredentialsType) { // cred_type == "ssl" if (use_prod_roots) { gpr_once_init(&g_once_init_add_prod_ssl_provider, &AddProdSslType); channel_creds = testing::GetCredentialsProvider()->GetChannelCredentials( @@ -95,10 +97,28 @@ std::shared_ptr CreateTestChannel( } return CreateCustomChannel(connect_to, channel_creds, channel_args); } else { - return CreateChannel(server, InsecureChannelCredentials()); + channel_creds = testing::GetCredentialsProvider()->GetChannelCredentials( + cred_type, &channel_args); + GPR_ASSERT(channel_creds != nullptr); + + return CreateChannel(server, channel_creds); } } +std::shared_ptr CreateTestChannel( + const grpc::string& server, const grpc::string& override_hostname, + bool enable_ssl, bool use_prod_roots, + const std::shared_ptr& creds, + const ChannelArguments& args) { + grpc::string type; + if (enable_ssl) { + type = testing::kTlsCredentialsType; + } + + return CreateTestChannel(server, type, override_hostname, use_prod_roots, + creds, args); +} + std::shared_ptr CreateTestChannel( const grpc::string& server, const grpc::string& override_hostname, bool enable_ssl, bool use_prod_roots, diff --git a/test/cpp/util/create_test_channel.h b/test/cpp/util/create_test_channel.h index 9b4b09171ef..e2ca8f99b40 100644 --- a/test/cpp/util/create_test_channel.h +++ b/test/cpp/util/create_test_channel.h @@ -44,6 +44,12 @@ std::shared_ptr CreateTestChannel( const std::shared_ptr& creds, const ChannelArguments& args); +std::shared_ptr CreateTestChannel( + const grpc::string& server, const grpc::string& cred_type, + const grpc::string& override_hostname, bool use_prod_roots, + const std::shared_ptr& creds, + const ChannelArguments& args); + std::shared_ptr CreateTestChannel( const grpc::string& server, const grpc::string& credential_type, const std::shared_ptr& creds); From 5b48dea50e93d865e6dbf0147e82648d79e72d8a Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 12 Jul 2017 19:00:35 -0700 Subject: [PATCH 03/26] Fix fd_orphan in ev_epollsig_linux --- src/core/lib/iomgr/ev_epollsig_linux.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/core/lib/iomgr/ev_epollsig_linux.c b/src/core/lib/iomgr/ev_epollsig_linux.c index 3c4ca9c7c59..0d969dccce9 100644 --- a/src/core/lib/iomgr/ev_epollsig_linux.c +++ b/src/core/lib/iomgr/ev_epollsig_linux.c @@ -855,24 +855,12 @@ static int fd_wrapped_fd(grpc_fd *fd) { static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, grpc_closure *on_done, int *release_fd, const char *reason) { - bool is_fd_closed = false; grpc_error *error = GRPC_ERROR_NONE; polling_island *unref_pi = NULL; gpr_mu_lock(&fd->po.mu); fd->on_done_closure = on_done; - /* If release_fd is not NULL, we should be relinquishing control of the file - descriptor fd->fd (but we still own the grpc_fd structure). */ - if (release_fd != NULL) { - *release_fd = fd->fd; - } else { - close(fd->fd); - is_fd_closed = true; - } - - fd->orphaned = true; - /* Remove the active status but keep referenced. We want this grpc_fd struct to be alive (and not added to freelist) until the end of this function */ REF_BY(fd, 1, reason); @@ -887,13 +875,24 @@ static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, before doing this.) */ if (fd->po.pi != NULL) { polling_island *pi_latest = polling_island_lock(fd->po.pi); - polling_island_remove_fd_locked(pi_latest, fd, is_fd_closed, &error); + polling_island_remove_fd_locked(pi_latest, fd, false /* is_fd_closed */, + &error); gpr_mu_unlock(&pi_latest->mu); unref_pi = fd->po.pi; fd->po.pi = NULL; } + /* If release_fd is not NULL, we should be relinquishing control of the file + descriptor fd->fd (but we still own the grpc_fd structure). */ + if (release_fd != NULL) { + *release_fd = fd->fd; + } else { + close(fd->fd); + } + + fd->orphaned = true; + GRPC_CLOSURE_SCHED(exec_ctx, fd->on_done_closure, GRPC_ERROR_REF(error)); gpr_mu_unlock(&fd->po.mu); From 35e9d41b0266b8949120ea47d15bfae3e87d9b85 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 12 Jul 2017 19:29:35 -0700 Subject: [PATCH 04/26] Also fix ev_epoll_limited_pollers_linux --- .../iomgr/ev_epoll_limited_pollers_linux.c | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c index 9f82c480bc4..27b4892d1da 100644 --- a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c +++ b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c @@ -932,24 +932,12 @@ static int fd_wrapped_fd(grpc_fd *fd) { static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, grpc_closure *on_done, int *release_fd, const char *reason) { - bool is_fd_closed = false; grpc_error *error = GRPC_ERROR_NONE; polling_island *unref_pi = NULL; gpr_mu_lock(&fd->po.mu); fd->on_done_closure = on_done; - /* If release_fd is not NULL, we should be relinquishing control of the file - descriptor fd->fd (but we still own the grpc_fd structure). */ - if (release_fd != NULL) { - *release_fd = fd->fd; - } else { - close(fd->fd); - is_fd_closed = true; - } - - fd->orphaned = true; - /* Remove the active status but keep referenced. We want this grpc_fd struct to be alive (and not added to freelist) until the end of this function */ REF_BY(fd, 1, reason); @@ -964,13 +952,24 @@ static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, before doing this.) */ if (fd->po.pi != NULL) { polling_island *pi_latest = polling_island_lock(fd->po.pi); - polling_island_remove_fd_locked(pi_latest, fd, is_fd_closed, &error); + polling_island_remove_fd_locked(pi_latest, fd, false /* is_fd_closed */, + &error); gpr_mu_unlock(&pi_latest->mu); unref_pi = fd->po.pi; fd->po.pi = NULL; } + /* If release_fd is not NULL, we should be relinquishing control of the file + descriptor fd->fd (but we still own the grpc_fd structure). */ + if (release_fd != NULL) { + *release_fd = fd->fd; + } else { + close(fd->fd); + } + + fd->orphaned = true; + GRPC_CLOSURE_SCHED(exec_ctx, fd->on_done_closure, GRPC_ERROR_REF(error)); gpr_mu_unlock(&fd->po.mu); From 6b08cf4c81f93f31d31e6c36124c0aff7919b3e8 Mon Sep 17 00:00:00 2001 From: Ian Coolidge Date: Fri, 16 Jun 2017 12:56:23 -0700 Subject: [PATCH 05/26] Propagate alwayslink in the grpc_cc_library build rule template. --- bazel/grpc_build_system.bzl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 8057f27a123..f793cae56d1 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -25,7 +25,8 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], external_deps = [], deps = [], standalone = False, - language = "C++", testonly = False, visibility = None): + language = "C++", testonly = False, visibility = None, + alwayslink = 0): copts = [] if language.upper() == "C": copts = ["-std=c99"] @@ -40,7 +41,8 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], linkopts = ["-pthread"], includes = [ "include" - ] + ], + alwayslink = alwayslink, ) def grpc_proto_plugin(name, srcs = [], deps = []): From 13213419f6381625a9d41622b94a31a91c0fc9c0 Mon Sep 17 00:00:00 2001 From: Ian Coolidge Date: Tue, 27 Jun 2017 21:49:44 -0700 Subject: [PATCH 06/26] alwayslink=1 for reflection plugin The reflection plugin uses a static initializer to enable itself, but no one depends on its symbols, so it gets optimized out. Set alwayslink in the reflection plugin to fix that. --- BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/BUILD b/BUILD index 26981ca35da..74d9a77cfeb 100644 --- a/BUILD +++ b/BUILD @@ -1522,6 +1522,7 @@ grpc_cc_library( ":grpc++", "//src/proto/grpc/reflection/v1alpha:reflection_proto", ], + alwayslink = 1, ) grpc_cc_library( From f47c063d021e8850e9cb393641981dfe3abdc1a1 Mon Sep 17 00:00:00 2001 From: Ken Payson Date: Thu, 13 Jul 2017 18:26:34 -0700 Subject: [PATCH 07/26] Restore interop_client_main This target is used internally. --- test/cpp/interop/BUILD | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/cpp/interop/BUILD b/test/cpp/interop/BUILD index 0de5a6f4da4..9123bd929ec 100644 --- a/test/cpp/interop/BUILD +++ b/test/cpp/interop/BUILD @@ -88,13 +88,22 @@ grpc_cc_library( ], ) -grpc_cc_binary( - name = "interop_client", +grpc_cc_library( + name = "interop_client_main", srcs = [ "client.cc", ], + language = "C++", deps = [ ":client_helper_lib", + ], +) + +grpc_cc_binary( + name = "interop_client", + language = "C++", + deps = [ + ":interop_client_main", "//:grpc++", ], ) From 0696611fb574e4d186ac8824ddeb37e8fd7838cf Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 14 Jul 2017 07:18:39 -0700 Subject: [PATCH 08/26] Do not return calls on server when request proto fails to deserialize. --- CMakeLists.txt | 57 +++++ Makefile | 55 +++++ build.yaml | 15 ++ include/grpc++/impl/codegen/core_codegen.h | 4 + .../impl/codegen/core_codegen_interface.h | 4 + .../grpc++/impl/codegen/server_interface.h | 41 +++- src/cpp/common/core_codegen.cc | 5 + test/cpp/server/BUILD | 43 ++++ test/cpp/server/server_request_call_test.cc | 162 +++++++++++++ .../generated/sources_and_headers.json | 26 ++ tools/run_tests/generated/tests.json | 22 ++ .../server_request_call_test.vcxproj | 223 ++++++++++++++++++ .../server_request_call_test.vcxproj.filters | 39 +++ 13 files changed, 689 insertions(+), 7 deletions(-) create mode 100644 test/cpp/server/BUILD create mode 100644 test/cpp/server/server_request_call_test.cc create mode 100644 vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj create mode 100644 vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj.filters diff --git a/CMakeLists.txt b/CMakeLists.txt index 3dd8ec45037..28aec791ee1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -748,6 +748,7 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx server_crash_test) endif() add_dependencies(buildtests_cxx server_crash_test_client) +add_dependencies(buildtests_cxx server_request_call_test) add_dependencies(buildtests_cxx shutdown_test) add_dependencies(buildtests_cxx status_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -11990,6 +11991,62 @@ target_link_libraries(server_crash_test_client endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(server_request_call_test + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.h + test/cpp/server/server_request_call_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +protobuf_generate_grpc_cpp( + src/proto/grpc/testing/echo_messages.proto +) +protobuf_generate_grpc_cpp( + src/proto/grpc/testing/echo.proto +) + +target_include_directories(server_request_call_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${BORINGSSL_ROOT_DIR}/include + PRIVATE ${PROTOBUF_ROOT_DIR}/src + PRIVATE ${BENCHMARK_ROOT_DIR}/include + PRIVATE ${ZLIB_ROOT_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib + PRIVATE ${CARES_BUILD_INCLUDE_DIR} + PRIVATE ${CARES_INCLUDE_DIR} + PRIVATE ${CARES_PLATFORM_INCLUDE_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include + PRIVATE third_party/googletest/googletest/include + PRIVATE third_party/googletest/googletest + PRIVATE third_party/googletest/googlemock/include + PRIVATE third_party/googletest/googlemock + PRIVATE ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(server_request_call_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc++_test_util + grpc_test_util + gpr_test_util + grpc++ + grpc + gpr + ${_gRPC_GFLAGS_LIBRARIES} +) + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(shutdown_test test/cpp/end2end/shutdown_test.cc third_party/googletest/googletest/src/gtest-all.cc diff --git a/Makefile b/Makefile index f58a02df9ae..ad9a93f2812 100644 --- a/Makefile +++ b/Makefile @@ -1166,6 +1166,7 @@ server_builder_test: $(BINDIR)/$(CONFIG)/server_builder_test server_context_test_spouse_test: $(BINDIR)/$(CONFIG)/server_context_test_spouse_test server_crash_test: $(BINDIR)/$(CONFIG)/server_crash_test server_crash_test_client: $(BINDIR)/$(CONFIG)/server_crash_test_client +server_request_call_test: $(BINDIR)/$(CONFIG)/server_request_call_test shutdown_test: $(BINDIR)/$(CONFIG)/shutdown_test status_test: $(BINDIR)/$(CONFIG)/status_test streaming_throughput_test: $(BINDIR)/$(CONFIG)/streaming_throughput_test @@ -1589,6 +1590,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/server_context_test_spouse_test \ $(BINDIR)/$(CONFIG)/server_crash_test \ $(BINDIR)/$(CONFIG)/server_crash_test_client \ + $(BINDIR)/$(CONFIG)/server_request_call_test \ $(BINDIR)/$(CONFIG)/shutdown_test \ $(BINDIR)/$(CONFIG)/status_test \ $(BINDIR)/$(CONFIG)/streaming_throughput_test \ @@ -1703,6 +1705,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/server_context_test_spouse_test \ $(BINDIR)/$(CONFIG)/server_crash_test \ $(BINDIR)/$(CONFIG)/server_crash_test_client \ + $(BINDIR)/$(CONFIG)/server_request_call_test \ $(BINDIR)/$(CONFIG)/shutdown_test \ $(BINDIR)/$(CONFIG)/status_test \ $(BINDIR)/$(CONFIG)/streaming_throughput_test \ @@ -2095,6 +2098,8 @@ test_cxx: buildtests_cxx $(Q) $(BINDIR)/$(CONFIG)/server_context_test_spouse_test || ( echo test server_context_test_spouse_test failed ; exit 1 ) $(E) "[RUN] Testing server_crash_test" $(Q) $(BINDIR)/$(CONFIG)/server_crash_test || ( echo test server_crash_test failed ; exit 1 ) + $(E) "[RUN] Testing server_request_call_test" + $(Q) $(BINDIR)/$(CONFIG)/server_request_call_test || ( echo test server_request_call_test failed ; exit 1 ) $(E) "[RUN] Testing shutdown_test" $(Q) $(BINDIR)/$(CONFIG)/shutdown_test || ( echo test shutdown_test failed ; exit 1 ) $(E) "[RUN] Testing status_test" @@ -15961,6 +15966,56 @@ endif endif +SERVER_REQUEST_CALL_TEST_SRC = \ + $(GENDIR)/src/proto/grpc/testing/echo_messages.pb.cc $(GENDIR)/src/proto/grpc/testing/echo_messages.grpc.pb.cc \ + $(GENDIR)/src/proto/grpc/testing/echo.pb.cc $(GENDIR)/src/proto/grpc/testing/echo.grpc.pb.cc \ + test/cpp/server/server_request_call_test.cc \ + +SERVER_REQUEST_CALL_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(SERVER_REQUEST_CALL_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/server_request_call_test: openssl_dep_error + +else + + + + +ifeq ($(NO_PROTOBUF),true) + +# You can't build the protoc plugins or protobuf-enabled targets if you don't have protobuf 3.0.0+. + +$(BINDIR)/$(CONFIG)/server_request_call_test: protobuf_dep_error + +else + +$(BINDIR)/$(CONFIG)/server_request_call_test: $(PROTOBUF_DEP) $(SERVER_REQUEST_CALL_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LDXX) $(LDFLAGS) $(SERVER_REQUEST_CALL_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/server_request_call_test + +endif + +endif + +$(OBJDIR)/$(CONFIG)/src/proto/grpc/testing/echo_messages.o: $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + +$(OBJDIR)/$(CONFIG)/src/proto/grpc/testing/echo.o: $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + +$(OBJDIR)/$(CONFIG)/test/cpp/server/server_request_call_test.o: $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_server_request_call_test: $(SERVER_REQUEST_CALL_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(SERVER_REQUEST_CALL_TEST_OBJS:.o=.dep) +endif +endif +$(OBJDIR)/$(CONFIG)/test/cpp/server/server_request_call_test.o: $(GENDIR)/src/proto/grpc/testing/echo_messages.pb.cc $(GENDIR)/src/proto/grpc/testing/echo_messages.grpc.pb.cc $(GENDIR)/src/proto/grpc/testing/echo.pb.cc $(GENDIR)/src/proto/grpc/testing/echo.grpc.pb.cc + + SHUTDOWN_TEST_SRC = \ test/cpp/end2end/shutdown_test.cc \ diff --git a/build.yaml b/build.yaml index 8a2fbe19077..375d6e1f6b2 100644 --- a/build.yaml +++ b/build.yaml @@ -4359,6 +4359,21 @@ targets: - grpc - gpr_test_util - gpr +- name: server_request_call_test + gtest: true + build: test + language: c++ + src: + - src/proto/grpc/testing/echo_messages.proto + - src/proto/grpc/testing/echo.proto + - test/cpp/server/server_request_call_test.cc + deps: + - grpc++_test_util + - grpc_test_util + - gpr_test_util + - grpc++ + - grpc + - gpr - name: shutdown_test gtest: true build: test diff --git a/include/grpc++/impl/codegen/core_codegen.h b/include/grpc++/impl/codegen/core_codegen.h index 504c79cff94..2b15a018455 100644 --- a/include/grpc++/impl/codegen/core_codegen.h +++ b/include/grpc++/impl/codegen/core_codegen.h @@ -60,6 +60,10 @@ class CoreCodegen final : public CoreCodegenInterface { void gpr_cv_signal(gpr_cv* cv) override; void gpr_cv_broadcast(gpr_cv* cv) override; + grpc_call_error grpc_call_cancel_with_status(grpc_call* call, + grpc_status_code status, + const char* description, + void* reserved) override; void grpc_call_ref(grpc_call* call) override; void grpc_call_unref(grpc_call* call) override; virtual void* grpc_call_arena_alloc(grpc_call* call, size_t length) override; diff --git a/include/grpc++/impl/codegen/core_codegen_interface.h b/include/grpc++/impl/codegen/core_codegen_interface.h index 930b116ddac..b4c771ac93a 100644 --- a/include/grpc++/impl/codegen/core_codegen_interface.h +++ b/include/grpc++/impl/codegen/core_codegen_interface.h @@ -89,6 +89,10 @@ class CoreCodegenInterface { virtual grpc_slice grpc_slice_new_with_user_data(void* p, size_t len, void (*destroy)(void*), void* user_data) = 0; + virtual grpc_call_error grpc_call_cancel_with_status(grpc_call* call, + grpc_status_code status, + const char* description, + void* reserved) = 0; virtual void grpc_call_ref(grpc_call* call) = 0; virtual void grpc_call_unref(grpc_call* call) = 0; virtual void* grpc_call_arena_alloc(grpc_call* call, size_t length) = 0; diff --git a/include/grpc++/impl/codegen/server_interface.h b/include/grpc++/impl/codegen/server_interface.h index 9d120031ca3..3e013a32b1c 100644 --- a/include/grpc++/impl/codegen/server_interface.h +++ b/include/grpc++/impl/codegen/server_interface.h @@ -176,22 +176,49 @@ class ServerInterface : public internal::CallHook { ServerCompletionQueue* notification_cq, void* tag, Message* request) : RegisteredAsyncRequest(server, context, stream, call_cq, tag), + registered_method_(registered_method), + server_(server), + context_(context), + stream_(stream), + call_cq_(call_cq), + notification_cq_(notification_cq), + tag_(tag), request_(request) { IssueRequest(registered_method, &payload_, notification_cq); } bool FinalizeResult(void** tag, bool* status) override { - bool serialization_status = - *status && payload_ && - SerializationTraits::Deserialize(payload_, request_).ok(); - bool ret = RegisteredAsyncRequest::FinalizeResult(tag, status); - *status = serialization_status && *status; - return ret; + if (*status) { + if (payload_ == nullptr || + !SerializationTraits::Deserialize(payload_, request_) + .ok()) { + // If deserialization fails, we cancel the call and instantiate + // a new instance of ourselves to request another call. We then + // return false, which prevents the call from being returned to + // the application. + g_core_codegen_interface->grpc_call_cancel_with_status( + call_, GRPC_STATUS_INTERNAL, "Unable to parse request", nullptr); + g_core_codegen_interface->grpc_call_unref(call_); + new PayloadAsyncRequest(registered_method_, server_, context_, + stream_, call_cq_, notification_cq_, tag_, + request_); + delete this; + return false; + } + } + return RegisteredAsyncRequest::FinalizeResult(tag, status); } private: - grpc_byte_buffer* payload_; + void* const registered_method_; + ServerInterface* const server_; + ServerContext* const context_; + internal::ServerAsyncStreamingInterface* const stream_; + CompletionQueue* const call_cq_; + ServerCompletionQueue* const notification_cq_; + void* const tag_; Message* const request_; + grpc_byte_buffer* payload_; }; class GenericAsyncRequest : public BaseAsyncRequest { diff --git a/src/cpp/common/core_codegen.cc b/src/cpp/common/core_codegen.cc index 71017407dea..c7c6b6b13b4 100644 --- a/src/cpp/common/core_codegen.cc +++ b/src/cpp/common/core_codegen.cc @@ -93,6 +93,11 @@ void CoreCodegen::grpc_byte_buffer_destroy(grpc_byte_buffer* bb) { ::grpc_byte_buffer_destroy(bb); } +grpc_call_error CoreCodegen::grpc_call_cancel_with_status( + grpc_call* call, grpc_status_code status, const char* description, + void* reserved) { + return ::grpc_call_cancel_with_status(call, status, description, reserved); +} void CoreCodegen::grpc_call_ref(grpc_call* call) { ::grpc_call_ref(call); } void CoreCodegen::grpc_call_unref(grpc_call* call) { ::grpc_call_unref(call); } void* CoreCodegen::grpc_call_arena_alloc(grpc_call* call, size_t length) { diff --git a/test/cpp/server/BUILD b/test/cpp/server/BUILD new file mode 100644 index 00000000000..512241e3504 --- /dev/null +++ b/test/cpp/server/BUILD @@ -0,0 +1,43 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +licenses(["notice"]) # Apache v2 + +load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_cc_library", "grpc_cc_binary") + +grpc_cc_test( + name = "server_builder_test", + srcs = ["server_builder_test.cc"], + deps = [ + "//:grpc++", + "//src/proto/grpc/testing:echo_proto", + "//test/core/util:grpc_test_util", + ], + external_deps = [ + "gtest", + ], +) + +grpc_cc_test( + name = "server_request_call_test", + srcs = ["server_request_call_test.cc"], + deps = [ + "//:grpc++", + "//src/proto/grpc/testing:echo_proto", + "//test/core/util:grpc_test_util", + ], + external_deps = [ + "gtest", + ], +) diff --git a/test/cpp/server/server_request_call_test.cc b/test/cpp/server/server_request_call_test.cc new file mode 100644 index 00000000000..1c64ddfc67c --- /dev/null +++ b/test/cpp/server/server_request_call_test.cc @@ -0,0 +1,162 @@ +/* + * + * Copyright 2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include + +#include +#include + +#include +#include + +#include +#include + +#include + +#include "src/proto/grpc/testing/echo.grpc.pb.h" +#include "test/core/util/port.h" + +namespace grpc { +namespace { + +TEST(ServerRequestCallTest, ShortDeadlineDoesNotCauseOkayFalse) { + std::mutex mu; + bool shutting_down = false; + + // grpc server config. + std::ostringstream s; + int p = grpc_pick_unused_port_or_die(); + s << "[::1]:" << p; + const string address = s.str(); + testing::EchoTestService::AsyncService service; + ServerBuilder builder; + builder.AddListeningPort(address, InsecureServerCredentials()); + auto cq = builder.AddCompletionQueue(); + builder.RegisterService(&service); + auto server = builder.BuildAndStart(); + + // server thread. + std::thread t([address, &service, &cq, &mu, &shutting_down] { + for (int n = 0; true; n++) { + ServerContext ctx; + testing::EchoRequest req; + ServerAsyncResponseWriter responder(&ctx); + + // if shutting down, don't enqueue a new request. + { + std::lock_guard lock(mu); + if (!shutting_down) { + service.RequestEcho(&ctx, &req, &responder, cq.get(), cq.get(), + (void*)1); + } + } + + bool ok; + void* tag; + if (!cq->Next(&tag, &ok)) { + break; + } + + EXPECT_EQ((void*)1, tag); + // If not shutting down, ok must be true for new requests. + { + std::lock_guard lock(mu); + if (!shutting_down && !ok) { + gpr_log(GPR_INFO, "!ok on request %d", n); + abort(); + } + if (shutting_down && !ok) { + // Failed connection due to shutdown, continue flushing the CQ. + continue; + } + } + + // Send a simple response after a small delay that would ensure the client + // deadline is exceeded. + gpr_log(GPR_INFO, "Got request %d", n); + testing::EchoResponse response; + response.set_message("foobar"); + // A bit of sleep to make sure the deadline elapses. + gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), + gpr_time_from_millis(50, GPR_TIMESPAN))); + { + std::lock_guard lock(mu); + if (shutting_down) { + gpr_log(GPR_INFO, + "shut down while processing call, not calling Finish()"); + // Continue flushing the CQ. + continue; + } + gpr_log(GPR_INFO, "Finishing request %d", n); + responder.Finish(response, grpc::Status::OK, (void*)2); + if (!cq->Next(&tag, &ok)) { + break; + } + EXPECT_EQ((void*)2, tag); + } + } + }); + + auto stub = testing::EchoTestService::NewStub( + CreateChannel(address, InsecureChannelCredentials())); + + for (int i = 0; i < 100; i++) { + gpr_log(GPR_INFO, "Sending %d.", i); + testing::EchoRequest request; + + ///////// + // Comment out the following line to get ok=false due to invalid request. + // Otherwise, ok=false due to deadline being exceeded. + ///////// + request.set_message("foobar"); + + // A simple request with a short deadline. The server will always exceed the + // deadline, whether due to the sleep or because the server was unable to + // even fetch the request from the CQ before the deadline elapsed. + testing::EchoResponse response; + ::grpc::ClientContext ctx; + ctx.set_fail_fast(false); + ctx.set_deadline(std::chrono::system_clock::now() + + std::chrono::milliseconds(1)); + grpc::Status status = stub->Echo(&ctx, request, &response); + EXPECT_EQ(DEADLINE_EXCEEDED, status.error_code()); + gpr_log(GPR_INFO, "Success."); + } + gpr_log(GPR_INFO, "Done sending RPCs."); + + // Shut down everything properly. + gpr_log(GPR_INFO, "Shutting down."); + { + std::lock_guard lock(mu); + shutting_down = true; + } + server->Shutdown(); + cq->Shutdown(); + server->Wait(); + + t.join(); +} + +} // namespace +} // namespace grpc + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 02562bfae4d..57ce5aa9fa4 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -3933,6 +3933,32 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc++", + "grpc++_test_util", + "grpc_test_util" + ], + "headers": [ + "src/proto/grpc/testing/echo.grpc.pb.h", + "src/proto/grpc/testing/echo.pb.h", + "src/proto/grpc/testing/echo_messages.grpc.pb.h", + "src/proto/grpc/testing/echo_messages.pb.h", + "src/proto/grpc/testing/echo_messages_mock.grpc.pb.h", + "src/proto/grpc/testing/echo_mock.grpc.pb.h" + ], + "is_filegroup": false, + "language": "c++", + "name": "server_request_call_test", + "src": [ + "test/cpp/server/server_request_call_test.cc" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "gpr", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index e44bf0e06d7..0036e23e4d0 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -3841,6 +3841,28 @@ "posix" ] }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "server_request_call_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ] + }, { "args": [], "ci_platforms": [ diff --git a/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj b/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj new file mode 100644 index 00000000000..55511759353 --- /dev/null +++ b/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj @@ -0,0 +1,223 @@ + + + + + + Debug + Win32 + + + Debug + x64 + + + Release + Win32 + + + Release + x64 + + + + {F33164EE-1406-7E49-E894-7E795146B882} + true + $(SolutionDir)IntDir\$(MSBuildProjectName)\ + + + + v100 + + + v110 + + + v120 + + + v140 + + + Application + true + Unicode + + + Application + false + true + Unicode + + + + + + + + + + + + + + + + server_request_call_test + static + Debug + static + Debug + + + server_request_call_test + static + Release + static + Release + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + Disabled + WIN32;_DEBUG;_LIB;%(PreprocessorDefinitions) + true + MultiThreadedDebug + true + None + false + + + Console + true + false + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + NotUsing + Level3 + MaxSpeed + WIN32;NDEBUG;_LIB;%(PreprocessorDefinitions) + true + true + true + MultiThreaded + true + None + false + + + Console + true + false + true + true + + + + + + + + + + + + + + + + + + + + + + + + + + {0BE77741-552A-929B-A497-4EF7ECE17A64} + + + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + + + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + + + {C187A093-A0FE-489D-A40A-6E33DE0F9FEB} + + + {29D16885-7228-4C31-81ED-5F9187C7F2A9} + + + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + + + + + + + + + + + + + + + This project references NuGet package(s) that are missing on this computer. Enable NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. + + + + + + + + + diff --git a/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj.filters b/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj.filters new file mode 100644 index 00000000000..a82cfb16d5d --- /dev/null +++ b/vsprojects/vcxproj/test/server_request_call_test/server_request_call_test.vcxproj.filters @@ -0,0 +1,39 @@ + + + + + src\proto\grpc\testing + + + src\proto\grpc\testing + + + test\cpp\server + + + + + + {48583c1d-014b-ecf7-ece7-98145537c913} + + + {f931e5e2-7d0e-a8d9-b072-1ed8095387a3} + + + {55f7d797-a139-d9c5-8cc3-7fde09c1d28b} + + + {1b39e313-5219-fbc1-9d88-9154b406e4ba} + + + {613e68e0-6bd4-3936-f8c2-34e255688225} + + + {871d6909-5ab9-336d-41ea-380563fcd3b3} + + + {4939f87e-df84-7c23-cd10-c23c06c72683} + + + + From ac9fdfbc40507a6c5453b171561537ee9cafe61a Mon Sep 17 00:00:00 2001 From: Guantao Liu Date: Fri, 14 Jul 2017 17:33:19 -0700 Subject: [PATCH 09/26] Removed unnecessary default values and fixed sanity. --- test/cpp/qps/BUILD | 3 ++ test/cpp/qps/driver.cc | 34 ++++++++----------- test/cpp/qps/driver.h | 6 ++-- test/cpp/qps/qps_json_driver.cc | 8 ++--- test/cpp/qps/qps_openloop_test.cc | 5 +-- test/cpp/qps/qps_worker.cc | 8 ++--- test/cpp/qps/qps_worker.h | 4 +-- .../qps/secure_sync_unary_ping_pong_test.cc | 5 +-- test/cpp/qps/worker.cc | 6 ++-- 9 files changed, 40 insertions(+), 39 deletions(-) diff --git a/test/cpp/qps/BUILD b/test/cpp/qps/BUILD index e24aa0cd0a0..b3348b76fab 100644 --- a/test/cpp/qps/BUILD +++ b/test/cpp/qps/BUILD @@ -149,6 +149,7 @@ grpc_cc_binary( ":driver_impl", "//:grpc++", "//test/cpp/util:test_config", + "//test/cpp/util:test_util", ], external_deps = [ "gflags", @@ -163,6 +164,7 @@ grpc_cc_test( ":driver_impl", ":qps_worker_impl", "//test/cpp/util:test_config", + "//test/cpp/util:test_util", ], ) @@ -174,6 +176,7 @@ grpc_cc_test( ":driver_impl", "//:grpc++", "//test/cpp/util:test_config", + "//test/cpp/util:test_util", ], ) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index bb4e7536e84..88d4d52abc0 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -175,13 +175,11 @@ static void postprocess_scenario_result(ScenarioResult* result) { sum(result->server_stats(), SvrPollCount) / histogram.Count()); auto server_queries_per_cpu_sec = - histogram.Count() / - (sum(result->server_stats(), ServerSystemTime) + - sum(result->server_stats(), ServerUserTime)); + histogram.Count() / (sum(result->server_stats(), ServerSystemTime) + + sum(result->server_stats(), ServerUserTime)); auto client_queries_per_cpu_sec = - histogram.Count() / - (sum(result->client_stats(), SystemTime) + - sum(result->client_stats(), UserTime)); + histogram.Count() / (sum(result->client_stats(), SystemTime) + + sum(result->client_stats(), UserTime)); result->mutable_summary()->set_server_queries_per_cpu_sec( server_queries_per_cpu_sec); @@ -193,7 +191,8 @@ std::unique_ptr RunScenario( const ClientConfig& initial_client_config, size_t num_clients, const ServerConfig& initial_server_config, size_t num_servers, int warmup_seconds, int benchmark_seconds, int spawn_local_worker_count, - const char* qps_server_target_override, const char* credential_type) { + const grpc::string& qps_server_target_override, + const grpc::string& credential_type) { // Log everything from the driver gpr_set_log_verbosity(GPR_LOG_SEVERITY_DEBUG); @@ -266,10 +265,9 @@ std::unique_ptr RunScenario( for (size_t i = 0; i < num_servers; i++) { gpr_log(GPR_INFO, "Starting server on %s (worker #%" PRIuPTR ")", workers[i].c_str(), i); - servers[i].stub = WorkerService::NewStub( - CreateChannel(workers[i], - GetCredentialsProvider()->GetChannelCredentials( - credential_type, &channel_args))); + servers[i].stub = WorkerService::NewStub(CreateChannel( + workers[i], GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); ServerConfig server_config = initial_server_config; if (server_config.core_limit() != 0) { @@ -316,9 +314,8 @@ std::unique_ptr RunScenario( gpr_log(GPR_INFO, "Starting client on %s (worker #%" PRIuPTR ")", worker.c_str(), i + num_servers); clients[i].stub = WorkerService::NewStub( - CreateChannel(worker, - GetCredentialsProvider()->GetChannelCredentials( - credential_type, &channel_args))); + CreateChannel(worker, GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); ClientConfig per_client_config = client_config; if (initial_client_config.core_limit() != 0) { @@ -503,7 +500,7 @@ std::unique_ptr RunScenario( return result; } -bool RunQuit(const char* credential_type) { +bool RunQuit(const grpc::string& credential_type) { // Get client, server lists bool result = true; auto workers = get_workers("QPS_WORKERS"); @@ -513,10 +510,9 @@ bool RunQuit(const char* credential_type) { ChannelArguments channel_args; for (size_t i = 0; i < workers.size(); i++) { - auto stub = WorkerService::NewStub( - CreateChannel(workers[i], - GetCredentialsProvider()->GetChannelCredentials( - credential_type, &channel_args))); + auto stub = WorkerService::NewStub(CreateChannel( + workers[i], GetCredentialsProvider()->GetChannelCredentials( + credential_type, &channel_args))); Void dummy; grpc::ClientContext ctx; ctx.set_wait_for_ready(true); diff --git a/test/cpp/qps/driver.h b/test/cpp/qps/driver.h index ddf75ce5872..29f2776d791 100644 --- a/test/cpp/qps/driver.h +++ b/test/cpp/qps/driver.h @@ -31,10 +31,10 @@ std::unique_ptr RunScenario( const grpc::testing::ClientConfig& client_config, size_t num_clients, const grpc::testing::ServerConfig& server_config, size_t num_servers, int warmup_seconds, int benchmark_seconds, int spawn_local_worker_count, - const char* qps_server_target_override = "", - const char* credential_type = "INSECURE_CREDENTIALS"); + const grpc::string& qps_server_target_override, + const grpc::string& credential_type); -bool RunQuit(const char* credential_type = "INSECURE_CREDENTIALS"); +bool RunQuit(const grpc::string& credential_type); } // namespace testing } // namespace grpc diff --git a/test/cpp/qps/qps_json_driver.cc b/test/cpp/qps/qps_json_driver.cc index d4698a7dc2f..cca59f64d8c 100644 --- a/test/cpp/qps/qps_json_driver.cc +++ b/test/cpp/qps/qps_json_driver.cc @@ -31,6 +31,7 @@ #include "test/cpp/qps/parse_json.h" #include "test/cpp/qps/report.h" #include "test/cpp/util/test_config.h" +#include "test/cpp/util/test_credentials_provider.h" DEFINE_string(scenarios_file, "", "JSON file containing an array of Scenario objects"); @@ -61,7 +62,7 @@ DEFINE_string(qps_server_target_override, "", DEFINE_string(json_file_out, "", "File to write the JSON output to."); -DEFINE_string(credential_type, "INSECURE_CREDENTIALS", +DEFINE_string(credential_type, grpc::testing::kInsecureCredentialsType, "Credential type for communication with workers"); namespace grpc { @@ -75,8 +76,7 @@ static std::unique_ptr RunAndReport(const Scenario& scenario, scenario.server_config(), scenario.num_servers(), scenario.warmup_seconds(), scenario.benchmark_seconds(), scenario.spawn_local_worker_count(), - FLAGS_qps_server_target_override.c_str(), - FLAGS_credential_type.c_str()); + FLAGS_qps_server_target_override, FLAGS_credential_type); // Amend the result with scenario config. Eventually we should adjust // RunScenario contract so we don't need to touch the result here. @@ -190,7 +190,7 @@ static bool QpsDriver() { } else if (scjson) { json = FLAGS_scenarios_json.c_str(); } else if (FLAGS_quit) { - return RunQuit(FLAGS_credential_type.c_str()); + return RunQuit(FLAGS_credential_type); } // Parse into an array of scenarios diff --git a/test/cpp/qps/qps_openloop_test.cc b/test/cpp/qps/qps_openloop_test.cc index 2f8a3d75f0c..069b3fa0768 100644 --- a/test/cpp/qps/qps_openloop_test.cc +++ b/test/cpp/qps/qps_openloop_test.cc @@ -25,6 +25,7 @@ #include "test/cpp/qps/driver.h" #include "test/cpp/qps/report.h" #include "test/cpp/util/test_config.h" +#include "test/cpp/util/test_credentials_provider.h" namespace grpc { namespace testing { @@ -48,8 +49,8 @@ static void RunQPS() { server_config.set_server_type(ASYNC_SERVER); server_config.set_async_server_threads(8); - const auto result = - RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2); + const auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, + BENCHMARK, -2, "", kInsecureCredentialsType); GetReporter()->ReportQPSPerCore(*result); GetReporter()->ReportLatency(*result); diff --git a/test/cpp/qps/qps_worker.cc b/test/cpp/qps/qps_worker.cc index 4db4c868241..d20bc1b074e 100644 --- a/test/cpp/qps/qps_worker.cc +++ b/test/cpp/qps/qps_worker.cc @@ -265,7 +265,7 @@ class WorkerServiceImpl final : public WorkerService::Service { }; QpsWorker::QpsWorker(int driver_port, int server_port, - const char* credential_type) { + const grpc::string& credential_type) { impl_.reset(new WorkerServiceImpl(server_port, this)); gpr_atm_rel_store(&done_, static_cast(0)); @@ -273,9 +273,9 @@ QpsWorker::QpsWorker(int driver_port, int server_port, gpr_join_host_port(&server_address, "::", driver_port); ServerBuilder builder; - builder.AddListeningPort(server_address, - GetCredentialsProvider()->GetServerCredentials( - credential_type)); + builder.AddListeningPort( + server_address, + GetCredentialsProvider()->GetServerCredentials(credential_type)); builder.RegisterService(impl_.get()); gpr_free(server_address); diff --git a/test/cpp/qps/qps_worker.h b/test/cpp/qps/qps_worker.h index ab5e26a10e4..32ff98da9b7 100644 --- a/test/cpp/qps/qps_worker.h +++ b/test/cpp/qps/qps_worker.h @@ -33,8 +33,8 @@ class WorkerServiceImpl; class QpsWorker { public: - explicit QpsWorker(int driver_port, int server_port = 0, - const char* credential_type = "INSECURE_CREDENTIALS"); + explicit QpsWorker(int driver_port, int server_port, + const grpc::string& credential_type); ~QpsWorker(); bool Done() const; diff --git a/test/cpp/qps/secure_sync_unary_ping_pong_test.cc b/test/cpp/qps/secure_sync_unary_ping_pong_test.cc index 1ee6e374749..137b33ee25f 100644 --- a/test/cpp/qps/secure_sync_unary_ping_pong_test.cc +++ b/test/cpp/qps/secure_sync_unary_ping_pong_test.cc @@ -24,6 +24,7 @@ #include "test/cpp/qps/driver.h" #include "test/cpp/qps/report.h" #include "test/cpp/util/test_config.h" +#include "test/cpp/util/test_credentials_provider.h" namespace grpc { namespace testing { @@ -51,8 +52,8 @@ static void RunSynchronousUnaryPingPong() { client_config.mutable_security_params()->CopyFrom(security); server_config.mutable_security_params()->CopyFrom(security); - const auto result = - RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK, -2); + const auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, + BENCHMARK, -2, "", kInsecureCredentialsType); GetReporter()->ReportQPS(*result); GetReporter()->ReportLatency(*result); diff --git a/test/cpp/qps/worker.cc b/test/cpp/qps/worker.cc index 7a8036327f6..27010b73152 100644 --- a/test/cpp/qps/worker.cc +++ b/test/cpp/qps/worker.cc @@ -27,10 +27,11 @@ #include "test/cpp/qps/qps_worker.h" #include "test/cpp/util/test_config.h" +#include "test/cpp/util/test_credentials_provider.h" DEFINE_int32(driver_port, 0, "Port for communication with driver"); DEFINE_int32(server_port, 0, "Port for operation as a server"); -DEFINE_string(credential_type, "INSECURE_CREDENTIALS", +DEFINE_string(credential_type, grpc::testing::kInsecureCredentialsType, "Credential type for communication with driver"); static bool got_sigint = false; @@ -41,8 +42,7 @@ namespace grpc { namespace testing { static void RunServer() { - QpsWorker worker(FLAGS_driver_port, FLAGS_server_port, - FLAGS_credential_type.c_str()); + QpsWorker worker(FLAGS_driver_port, FLAGS_server_port, FLAGS_credential_type); while (!got_sigint && !worker.Done()) { gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), From 91d8f0b5a846553ed11d0908129aaad116f63be4 Mon Sep 17 00:00:00 2001 From: Guantao Liu Date: Fri, 14 Jul 2017 18:33:30 -0700 Subject: [PATCH 10/26] Added the missing header file and fixed a syntax error. --- test/cpp/qps/driver.cc | 3 +-- test/cpp/qps/qps_worker.h | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index 88d4d52abc0..4458e389e7e 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -285,8 +285,7 @@ std::unique_ptr RunScenario( if (!servers[i].stream->Read(&init_status)) { gpr_log(GPR_ERROR, "Server %zu did not yield initial status", i); } - if (qps_server_target_override != NULL && - strlen(qps_server_target_override) > 0) { + if (qps_server_target_override.length() > 0) { // overriding the qps server target only works if there is 1 server GPR_ASSERT(num_servers == 1); client_config.add_server_targets(qps_server_target_override); diff --git a/test/cpp/qps/qps_worker.h b/test/cpp/qps/qps_worker.h index 32ff98da9b7..5c1d1a8ae64 100644 --- a/test/cpp/qps/qps_worker.h +++ b/test/cpp/qps/qps_worker.h @@ -22,6 +22,7 @@ #include #include +#include namespace grpc { From 29a7050c357e64639c0f1bf699489659e34778ee Mon Sep 17 00:00:00 2001 From: Guantao Liu Date: Fri, 14 Jul 2017 19:24:26 -0700 Subject: [PATCH 11/26] Fixed sanity. --- test/cpp/qps/qps_worker.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/qps/qps_worker.h b/test/cpp/qps/qps_worker.h index 5c1d1a8ae64..360125fb170 100644 --- a/test/cpp/qps/qps_worker.h +++ b/test/cpp/qps/qps_worker.h @@ -21,8 +21,8 @@ #include -#include #include +#include namespace grpc { From 1435bfc718b2816d17ff52fbac011bf5ba971109 Mon Sep 17 00:00:00 2001 From: Yihua Zhang Date: Mon, 17 Jul 2017 11:20:51 -0700 Subject: [PATCH 12/26] Add GTS plugin --- BUILD | 3 ++ CMakeLists.txt | 2 + Makefile | 3 ++ binding.gyp | 1 + build.yaml | 4 ++ config.m4 | 1 + config.w32 | 1 + gRPC-Core.podspec | 3 ++ grpc.gemspec | 2 + package.xml | 2 + .../grpc_cronet_plugin_registry.c | 4 ++ .../plugin_registry/grpc_plugin_registry.c | 4 ++ src/core/tsi/gts_transport_security.c | 40 +++++++++++++++++++ src/core/tsi/gts_transport_security.h | 37 +++++++++++++++++ src/python/grpcio/grpc_core_dependencies.py | 1 + tools/doxygen/Doxyfile.core.internal | 2 + .../generated/sources_and_headers.json | 4 ++ vsprojects/vcxproj/grpc/grpc.vcxproj | 3 ++ vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 +++ 19 files changed, 123 insertions(+) create mode 100644 src/core/tsi/gts_transport_security.c create mode 100644 src/core/tsi/gts_transport_security.h diff --git a/BUILD b/BUILD index 6f92ef3858f..e8ef2cb2eed 100644 --- a/BUILD +++ b/BUILD @@ -1402,12 +1402,14 @@ grpc_cc_library( name = "tsi", srcs = [ "src/core/tsi/fake_transport_security.c", + "src/core/tsi/gts_transport_security.c", "src/core/tsi/ssl_transport_security.c", "src/core/tsi/transport_security.c", "src/core/tsi/transport_security_adapter.c", ], hdrs = [ "src/core/tsi/fake_transport_security.h", + "src/core/tsi/gts_transport_security.h", "src/core/tsi/ssl_transport_security.h", "src/core/tsi/ssl_types.h", "src/core/tsi/transport_security.h", @@ -1420,6 +1422,7 @@ grpc_cc_library( language = "c", deps = [ "gpr", + "grpc_base", "grpc_trace", ], ) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ee9490c250..e4243766bfa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1122,6 +1122,7 @@ add_library(grpc src/core/lib/security/util/json_util.c src/core/lib/surface/init_secure.c src/core/tsi/fake_transport_security.c + src/core/tsi/gts_transport_security.c src/core/tsi/ssl_transport_security.c src/core/tsi/transport_security.c src/core/tsi/transport_security_adapter.c @@ -1487,6 +1488,7 @@ add_library(grpc_cronet src/core/lib/security/util/json_util.c src/core/lib/surface/init_secure.c src/core/tsi/fake_transport_security.c + src/core/tsi/gts_transport_security.c src/core/tsi/ssl_transport_security.c src/core/tsi/transport_security.c src/core/tsi/transport_security_adapter.c diff --git a/Makefile b/Makefile index 72468f8f928..add767b0f15 100644 --- a/Makefile +++ b/Makefile @@ -3059,6 +3059,7 @@ LIBGRPC_SRC = \ src/core/lib/security/util/json_util.c \ src/core/lib/surface/init_secure.c \ src/core/tsi/fake_transport_security.c \ + src/core/tsi/gts_transport_security.c \ src/core/tsi/ssl_transport_security.c \ src/core/tsi/transport_security.c \ src/core/tsi/transport_security_adapter.c \ @@ -3422,6 +3423,7 @@ LIBGRPC_CRONET_SRC = \ src/core/lib/security/util/json_util.c \ src/core/lib/surface/init_secure.c \ src/core/tsi/fake_transport_security.c \ + src/core/tsi/gts_transport_security.c \ src/core/tsi/ssl_transport_security.c \ src/core/tsi/transport_security.c \ src/core/tsi/transport_security_adapter.c \ @@ -19133,6 +19135,7 @@ src/core/lib/surface/init_secure.c: $(OPENSSL_DEP) src/core/plugin_registry/grpc_cronet_plugin_registry.c: $(OPENSSL_DEP) src/core/plugin_registry/grpc_plugin_registry.c: $(OPENSSL_DEP) src/core/tsi/fake_transport_security.c: $(OPENSSL_DEP) +src/core/tsi/gts_transport_security.c: $(OPENSSL_DEP) src/core/tsi/ssl_transport_security.c: $(OPENSSL_DEP) src/core/tsi/transport_security.c: $(OPENSSL_DEP) src/core/tsi/transport_security_adapter.c: $(OPENSSL_DEP) diff --git a/binding.gyp b/binding.gyp index 00fbe70fee8..6bc6450bbd8 100644 --- a/binding.gyp +++ b/binding.gyp @@ -813,6 +813,7 @@ 'src/core/lib/security/util/json_util.c', 'src/core/lib/surface/init_secure.c', 'src/core/tsi/fake_transport_security.c', + 'src/core/tsi/gts_transport_security.c', 'src/core/tsi/ssl_transport_security.c', 'src/core/tsi/transport_security.c', 'src/core/tsi/transport_security_adapter.c', diff --git a/build.yaml b/build.yaml index 1b9eb63bcdc..adb1e7b947c 100644 --- a/build.yaml +++ b/build.yaml @@ -874,6 +874,7 @@ filegroups: - name: tsi headers: - src/core/tsi/fake_transport_security.h + - src/core/tsi/gts_transport_security.h - src/core/tsi/ssl_transport_security.h - src/core/tsi/ssl_types.h - src/core/tsi/transport_security.h @@ -881,14 +882,17 @@ filegroups: - src/core/tsi/transport_security_interface.h src: - src/core/tsi/fake_transport_security.c + - src/core/tsi/gts_transport_security.c - src/core/tsi/ssl_transport_security.c - src/core/tsi/transport_security.c - src/core/tsi/transport_security_adapter.c deps: - gpr + plugin: grpc_tsi_gts secure: true uses: - grpc_trace + - grpc_base - name: grpc++_base language: c++ public_headers: diff --git a/config.m4 b/config.m4 index 71e5776d851..ebe0a1b3a05 100644 --- a/config.m4 +++ b/config.m4 @@ -263,6 +263,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/security/util/json_util.c \ src/core/lib/surface/init_secure.c \ src/core/tsi/fake_transport_security.c \ + src/core/tsi/gts_transport_security.c \ src/core/tsi/ssl_transport_security.c \ src/core/tsi/transport_security.c \ src/core/tsi/transport_security_adapter.c \ diff --git a/config.w32 b/config.w32 index cd374bae650..e8ce70f158d 100644 --- a/config.w32 +++ b/config.w32 @@ -240,6 +240,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\security\\util\\json_util.c " + "src\\core\\lib\\surface\\init_secure.c " + "src\\core\\tsi\\fake_transport_security.c " + + "src\\core\\tsi\\gts_transport_security.c " + "src\\core\\tsi\\ssl_transport_security.c " + "src\\core\\tsi\\transport_security.c " + "src\\core\\tsi\\transport_security_adapter.c " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index fc51a92daf8..1d9eca36dc5 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -397,6 +397,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/transport/tsi_error.h', 'src/core/lib/security/util/json_util.h', 'src/core/tsi/fake_transport_security.h', + 'src/core/tsi/gts_transport_security.h', 'src/core/tsi/ssl_transport_security.h', 'src/core/tsi/ssl_types.h', 'src/core/tsi/transport_security.h', @@ -635,6 +636,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.c', 'src/core/lib/surface/init_secure.c', 'src/core/tsi/fake_transport_security.c', + 'src/core/tsi/gts_transport_security.c', 'src/core/tsi/ssl_transport_security.c', 'src/core/tsi/transport_security.c', 'src/core/tsi/transport_security_adapter.c', @@ -877,6 +879,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/transport/tsi_error.h', 'src/core/lib/security/util/json_util.h', 'src/core/tsi/fake_transport_security.h', + 'src/core/tsi/gts_transport_security.h', 'src/core/tsi/ssl_transport_security.h', 'src/core/tsi/ssl_types.h', 'src/core/tsi/transport_security.h', diff --git a/grpc.gemspec b/grpc.gemspec index ab4a634c9b0..dad1b31ec91 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -329,6 +329,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/security/transport/tsi_error.h ) s.files += %w( src/core/lib/security/util/json_util.h ) s.files += %w( src/core/tsi/fake_transport_security.h ) + s.files += %w( src/core/tsi/gts_transport_security.h ) s.files += %w( src/core/tsi/ssl_transport_security.h ) s.files += %w( src/core/tsi/ssl_types.h ) s.files += %w( src/core/tsi/transport_security.h ) @@ -571,6 +572,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/security/util/json_util.c ) s.files += %w( src/core/lib/surface/init_secure.c ) s.files += %w( src/core/tsi/fake_transport_security.c ) + s.files += %w( src/core/tsi/gts_transport_security.c ) s.files += %w( src/core/tsi/ssl_transport_security.c ) s.files += %w( src/core/tsi/transport_security.c ) s.files += %w( src/core/tsi/transport_security_adapter.c ) diff --git a/package.xml b/package.xml index 602800b1b51..c79b38f686c 100644 --- a/package.xml +++ b/package.xml @@ -343,6 +343,7 @@ + @@ -585,6 +586,7 @@ + diff --git a/src/core/plugin_registry/grpc_cronet_plugin_registry.c b/src/core/plugin_registry/grpc_cronet_plugin_registry.c index b468c03aa34..322ebea1115 100644 --- a/src/core/plugin_registry/grpc_cronet_plugin_registry.c +++ b/src/core/plugin_registry/grpc_cronet_plugin_registry.c @@ -26,6 +26,8 @@ extern void grpc_deadline_filter_init(void); extern void grpc_deadline_filter_shutdown(void); extern void grpc_client_channel_init(void); extern void grpc_client_channel_shutdown(void); +extern void grpc_tsi_gts_init(void); +extern void grpc_tsi_gts_shutdown(void); extern void grpc_load_reporting_plugin_init(void); extern void grpc_load_reporting_plugin_shutdown(void); @@ -38,6 +40,8 @@ void grpc_register_built_in_plugins(void) { grpc_deadline_filter_shutdown); grpc_register_plugin(grpc_client_channel_init, grpc_client_channel_shutdown); + grpc_register_plugin(grpc_tsi_gts_init, + grpc_tsi_gts_shutdown); grpc_register_plugin(grpc_load_reporting_plugin_init, grpc_load_reporting_plugin_shutdown); } diff --git a/src/core/plugin_registry/grpc_plugin_registry.c b/src/core/plugin_registry/grpc_plugin_registry.c index b0d06a3f43a..fa9974952c0 100644 --- a/src/core/plugin_registry/grpc_plugin_registry.c +++ b/src/core/plugin_registry/grpc_plugin_registry.c @@ -22,6 +22,8 @@ extern void grpc_http_filters_init(void); extern void grpc_http_filters_shutdown(void); extern void grpc_chttp2_plugin_init(void); extern void grpc_chttp2_plugin_shutdown(void); +extern void grpc_tsi_gts_init(void); +extern void grpc_tsi_gts_shutdown(void); extern void grpc_deadline_filter_init(void); extern void grpc_deadline_filter_shutdown(void); extern void grpc_client_channel_init(void); @@ -58,6 +60,8 @@ void grpc_register_built_in_plugins(void) { grpc_http_filters_shutdown); grpc_register_plugin(grpc_chttp2_plugin_init, grpc_chttp2_plugin_shutdown); + grpc_register_plugin(grpc_tsi_gts_init, + grpc_tsi_gts_shutdown); grpc_register_plugin(grpc_deadline_filter_init, grpc_deadline_filter_shutdown); grpc_register_plugin(grpc_client_channel_init, diff --git a/src/core/tsi/gts_transport_security.c b/src/core/tsi/gts_transport_security.c new file mode 100644 index 00000000000..e2ac685e441 --- /dev/null +++ b/src/core/tsi/gts_transport_security.c @@ -0,0 +1,40 @@ +/* + * + * Copyright 2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "src/core/tsi/gts_transport_security.h" + +#include + +static gts_shared_resource g_gts_resource; + +gts_shared_resource *gts_get_shared_resource(void) { return &g_gts_resource; } + +void grpc_tsi_gts_init() { + memset(&g_gts_resource, 0, sizeof(gts_shared_resource)); + gpr_mu_init(&g_gts_resource.mu); +} + +void grpc_tsi_gts_shutdown() { + gpr_mu_destroy(&g_gts_resource.mu); + if (g_gts_resource.cq == NULL) { + return; + } + grpc_completion_queue_destroy(g_gts_resource.cq); + grpc_channel_destroy(g_gts_resource.channel); + gpr_thd_join(g_gts_resource.thread_id); +} diff --git a/src/core/tsi/gts_transport_security.h b/src/core/tsi/gts_transport_security.h new file mode 100644 index 00000000000..538e1030bc5 --- /dev/null +++ b/src/core/tsi/gts_transport_security.h @@ -0,0 +1,37 @@ +/* + * + * Copyright 2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#ifndef GRPC_CORE_TSI_GTS_TRANSPORT_SECURITY_H +#define GRPC_CORE_TSI_GTS_TRANSPORT_SECURITY_H + +#include +#include +#include + +typedef struct gts_shared_resource { + gpr_thd_id thread_id; + grpc_channel *channel; + grpc_completion_queue *cq; + gpr_mu mu; +} gts_shared_resource; + +/* This method returns the address of gts_shared_resource object shared by all + * TSI handshakes. */ +gts_shared_resource *gts_get_shared_resource(void); + +#endif /* GRPC_CORE_TSI_GTS_TRANSPORT_SECURITY_H */ diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index b30d340ce56..17cde1ac8ff 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -239,6 +239,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/security/util/json_util.c', 'src/core/lib/surface/init_secure.c', 'src/core/tsi/fake_transport_security.c', + 'src/core/tsi/gts_transport_security.c', 'src/core/tsi/ssl_transport_security.c', 'src/core/tsi/transport_security.c', 'src/core/tsi/transport_security_adapter.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 4c6c656b1b5..bcdcab6c9ac 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1402,6 +1402,8 @@ src/core/plugin_registry/grpc_plugin_registry.c \ src/core/tsi/README.md \ src/core/tsi/fake_transport_security.c \ src/core/tsi/fake_transport_security.h \ +src/core/tsi/gts_transport_security.c \ +src/core/tsi/gts_transport_security.h \ src/core/tsi/ssl_transport_security.c \ src/core/tsi/ssl_transport_security.h \ src/core/tsi/ssl_types.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 0a91a0cec45..c9ecfe9f39e 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -8872,10 +8872,12 @@ { "deps": [ "gpr", + "grpc_base", "grpc_trace" ], "headers": [ "src/core/tsi/fake_transport_security.h", + "src/core/tsi/gts_transport_security.h", "src/core/tsi/ssl_transport_security.h", "src/core/tsi/ssl_types.h", "src/core/tsi/transport_security.h", @@ -8888,6 +8890,8 @@ "src": [ "src/core/tsi/fake_transport_security.c", "src/core/tsi/fake_transport_security.h", + "src/core/tsi/gts_transport_security.c", + "src/core/tsi/gts_transport_security.h", "src/core/tsi/ssl_transport_security.c", "src/core/tsi/ssl_transport_security.h", "src/core/tsi/ssl_types.h", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 112f3ec30ac..a7940833fb3 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -454,6 +454,7 @@ + @@ -876,6 +877,8 @@ + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 5c29e58badb..6857c3d8a1f 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -535,6 +535,9 @@ src\core\tsi + + src\core\tsi + src\core\tsi @@ -1313,6 +1316,9 @@ src\core\tsi + + src\core\tsi + src\core\tsi From 1c0b20dda793124f3b97978060ba13678b614d3e Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Mon, 17 Jul 2017 20:30:32 +0000 Subject: [PATCH 13/26] Narrow .gitignore to only what it needs to ignore "proto" and "*.egg-info/" are no longer generated, "src/" can be narrowed to "/src/", and "*_pb2.py" and "*_pb2_grpc.py" are covered by .gitignore files in parent directories. --- src/python/grpcio_tests/.gitignore | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/python/grpcio_tests/.gitignore b/src/python/grpcio_tests/.gitignore index dcba283a8ca..4f00cd9a736 100644 --- a/src/python/grpcio_tests/.gitignore +++ b/src/python/grpcio_tests/.gitignore @@ -1,5 +1 @@ -proto/ -src/ -*_pb2.py -*_pb2_grpc.py -*.egg-info/ +/src/ From 457b02e6b2ef32b2e57430250ac5fcb8d04237e1 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Mon, 17 Jul 2017 13:48:08 -0700 Subject: [PATCH 14/26] Fix image path when pulled from dockerhub. --- tools/run_tests/dockerize/build_interop_image.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/dockerize/build_interop_image.sh b/tools/run_tests/dockerize/build_interop_image.sh index 77c7a84828e..9d8ad5325c1 100755 --- a/tools/run_tests/dockerize/build_interop_image.sh +++ b/tools/run_tests/dockerize/build_interop_image.sh @@ -69,8 +69,8 @@ fi if [ "$DOCKERHUB_ORGANIZATION" != "" ] then - DOCKER_IMAGE_NAME=$DOCKERHUB_ORGANIZATION/$BASE_IMAGE - docker pull $DOCKER_IMAGE_NAME + BASE_IMAGE=$DOCKERHUB_ORGANIZATION/$BASE_IMAGE + docker pull $BASE_IMAGE else # Make sure docker image has been built. Should be instantaneous if so. docker build -t $BASE_IMAGE --force-rm=true tools/dockerfile/interoptest/$BASE_NAME || exit $? From a2eb6f7e701c7dff0eba6fed5a61c0f9d7034e8e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 18 Jul 2017 15:13:14 +0200 Subject: [PATCH 15/26] set path to mingw on kokoro --- tools/internal_ci/windows/grpc_build_artifacts.bat | 3 +++ tools/run_tests/artifacts/build_artifact_python.bat | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/internal_ci/windows/grpc_build_artifacts.bat b/tools/internal_ci/windows/grpc_build_artifacts.bat index d5ade5a83d5..8cfa2fb0bea 100644 --- a/tools/internal_ci/windows/grpc_build_artifacts.bat +++ b/tools/internal_ci/windows/grpc_build_artifacts.bat @@ -19,10 +19,13 @@ rename C:\Python34_32bit Python34_32bits rename C:\Python35_32bit Python35_32bits rename C:\Python36_32bit Python36_32bits +pacman -S --noconfirm mingw64/mingw-w64-x86_64-gcc mingw32/mingw-w64-i686-gcc + @rem make sure msys binaries are preferred over cygwin binaries @rem set path to python 2.7 set PATH=C:\tools\msys64\usr\bin;C:\Python27;%PATH% + @rem enter repo root cd /d %~dp0\..\..\.. diff --git a/tools/run_tests/artifacts/build_artifact_python.bat b/tools/run_tests/artifacts/build_artifact_python.bat index 4c34fc50f04..3a0564129cf 100644 --- a/tools/run_tests/artifacts/build_artifact_python.bat +++ b/tools/run_tests/artifacts/build_artifact_python.bat @@ -12,8 +12,8 @@ @rem See the License for the specific language governing permissions and @rem limitations under the License. - -set PATH=C:\%1;C:\%1\scripts;C:\msys64\mingw%2\bin;%PATH% +@rem set path to python & mingw compiler +set PATH=C:\%1;C:\%1\scripts;C:\msys64\mingw%2\bin;C:\tools\msys64\mingw%2\bin;%PATH% pip install --upgrade six pip install --upgrade setuptools From cb8eaaa88364fb3ff657e3c5bbf7f75d737859ec Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 18 Jul 2017 15:12:49 +0200 Subject: [PATCH 16/26] fix grpcio_tools build on kokoro --- tools/distrib/python/grpcio_tools/setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/distrib/python/grpcio_tools/setup.py b/tools/distrib/python/grpcio_tools/setup.py index bc24e4c462a..ddbb348e0cd 100644 --- a/tools/distrib/python/grpcio_tools/setup.py +++ b/tools/distrib/python/grpcio_tools/setup.py @@ -62,9 +62,9 @@ if EXTRA_ENV_COMPILE_ARGS is None: # envvars) without adding yet more GRPC-specific envvars. # See https://sourceforge.net/p/mingw-w64/bugs/363/ if '32' in platform.architecture()[0]: - EXTRA_ENV_COMPILE_ARGS += ' -D_ftime=_ftime32 -D_timeb=__timeb32 -D_ftime_s=_ftime32_s' + EXTRA_ENV_COMPILE_ARGS += ' -D_ftime=_ftime32 -D_timeb=__timeb32 -D_ftime_s=_ftime32_s -D_hypot=hypot' else: - EXTRA_ENV_COMPILE_ARGS += ' -D_ftime=_ftime64 -D_timeb=__timeb64' + EXTRA_ENV_COMPILE_ARGS += ' -D_ftime=_ftime64 -D_timeb=__timeb64 -D_hypot=hypot' else: # We need to statically link the C++ Runtime, only the C runtime is # available dynamically From 0f95fa4909ba6f65364cb0b5c8c37a27c09b67b4 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 18 Jul 2017 10:42:33 -0700 Subject: [PATCH 17/26] Add idiomatic C++ API for grpc::Slice construction that doesn't require using grpc_slice --- include/grpc++/support/slice.h | 14 ++++++++++ src/cpp/util/slice_cc.cc | 13 +++++++++ test/cpp/qps/client.h | 4 +-- test/cpp/util/byte_buffer_proto_helper.cc | 3 +-- test/cpp/util/byte_buffer_test.cc | 21 +++++++-------- test/cpp/util/cli_call.cc | 3 +-- test/cpp/util/slice_test.cc | 33 +++++++++++++++++++---- 7 files changed, 67 insertions(+), 24 deletions(-) diff --git a/include/grpc++/support/slice.h b/include/grpc++/support/slice.h index db5cf942a0c..0b4ba7cecb4 100644 --- a/include/grpc++/support/slice.h +++ b/include/grpc++/support/slice.h @@ -44,6 +44,20 @@ class Slice final { /// Construct a slice from \a slice, stealing a reference. Slice(grpc_slice slice, StealRef); + /// Allocate a slice of specified size + Slice(size_t len); + + /// Construct a slice from a copied buffer + Slice(const void* buf, size_t len); + + /// Construct a slice from a copied string + Slice(const grpc::string& str); + + enum StaticSlice { STATIC_SLICE }; + + /// Construct a slice from a static buffer + Slice(const void* buf, size_t len, StaticSlice); + /// Copy constructor, adds a reference. Slice(const Slice& other); diff --git a/src/cpp/util/slice_cc.cc b/src/cpp/util/slice_cc.cc index 80b4b3a4712..56e0328b949 100644 --- a/src/cpp/util/slice_cc.cc +++ b/src/cpp/util/slice_cc.cc @@ -28,6 +28,19 @@ Slice::Slice(grpc_slice slice, AddRef) : slice_(grpc_slice_ref(slice)) {} Slice::Slice(grpc_slice slice, StealRef) : slice_(slice) {} +Slice::Slice(size_t len) : slice_(grpc_slice_malloc(len)) {} + +Slice::Slice(const void* buf, size_t len) + : slice_(grpc_slice_from_copied_buffer(reinterpret_cast(buf), + len)) {} + +Slice::Slice(const grpc::string& str) + : slice_(grpc_slice_from_copied_buffer(str.c_str(), str.length())) {} + +Slice::Slice(const void* buf, size_t len, StaticSlice) + : slice_(grpc_slice_from_static_buffer(reinterpret_cast(buf), + len)) {} + Slice::Slice(const Slice& other) : slice_(grpc_slice_ref(other.slice_)) {} } // namespace grpc diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index e759446c1ae..ecbf9c31fa1 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -88,9 +88,7 @@ class ClientRequestCreator { if (payload_config.has_bytebuf_params()) { std::unique_ptr buf( new char[payload_config.bytebuf_params().req_size()]); - grpc_slice s = grpc_slice_from_copied_buffer( - buf.get(), payload_config.bytebuf_params().req_size()); - Slice slice(s, Slice::STEAL_REF); + Slice slice(buf.get(), payload_config.bytebuf_params().req_size()); *req = ByteBuffer(&slice, 1); } else { GPR_ASSERT(false); // not appropriate for this specialization diff --git a/test/cpp/util/byte_buffer_proto_helper.cc b/test/cpp/util/byte_buffer_proto_helper.cc index 22dd074fe93..bb5162f86e6 100644 --- a/test/cpp/util/byte_buffer_proto_helper.cc +++ b/test/cpp/util/byte_buffer_proto_helper.cc @@ -36,8 +36,7 @@ std::unique_ptr SerializeToByteBuffer( grpc::protobuf::Message* message) { grpc::string buf; message->SerializeToString(&buf); - grpc_slice s = grpc_slice_from_copied_string(buf.c_str()); - Slice slice(s, Slice::STEAL_REF); + Slice slice(buf); return std::unique_ptr(new ByteBuffer(&slice, 1)); } diff --git a/test/cpp/util/byte_buffer_test.cc b/test/cpp/util/byte_buffer_test.cc index 6c077ddb2c3..cac01a73078 100644 --- a/test/cpp/util/byte_buffer_test.cc +++ b/test/cpp/util/byte_buffer_test.cc @@ -34,33 +34,30 @@ const char* kContent2 = "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy world"; class ByteBufferTest : public ::testing::Test {}; TEST_F(ByteBufferTest, CreateFromSingleSlice) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - Slice s(hello, Slice::STEAL_REF); + Slice s(kContent1); ByteBuffer buffer(&s, 1); + EXPECT_EQ(strlen(kContent1), buffer.Length()); } TEST_F(ByteBufferTest, CreateFromVector) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - grpc_slice world = grpc_slice_from_copied_string(kContent2); std::vector slices; - slices.push_back(Slice(hello, Slice::STEAL_REF)); - slices.push_back(Slice(world, Slice::STEAL_REF)); + slices.emplace_back(kContent1); + slices.emplace_back(kContent2); ByteBuffer buffer(&slices[0], 2); + EXPECT_EQ(strlen(kContent1) + strlen(kContent2), buffer.Length()); } TEST_F(ByteBufferTest, Clear) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - Slice s(hello, Slice::STEAL_REF); + Slice s(kContent1); ByteBuffer buffer(&s, 1); buffer.Clear(); + EXPECT_EQ(static_cast(0), buffer.Length()); } TEST_F(ByteBufferTest, Length) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - grpc_slice world = grpc_slice_from_copied_string(kContent2); std::vector slices; - slices.push_back(Slice(hello, Slice::STEAL_REF)); - slices.push_back(Slice(world, Slice::STEAL_REF)); + slices.emplace_back(kContent1); + slices.emplace_back(kContent2); ByteBuffer buffer(&slices[0], 2); EXPECT_EQ(strlen(kContent1) + strlen(kContent2), buffer.Length()); } diff --git a/test/cpp/util/cli_call.cc b/test/cpp/util/cli_call.cc index fb6d76bb906..c3220efa544 100644 --- a/test/cpp/util/cli_call.cc +++ b/test/cpp/util/cli_call.cc @@ -119,8 +119,7 @@ void CliCall::WritesDone() { } void CliCall::WriteAndWait(const grpc::string& request) { - grpc_slice s = grpc_slice_from_copied_string(request.c_str()); - grpc::Slice req_slice(s, grpc::Slice::STEAL_REF); + grpc::Slice req_slice(request); grpc::ByteBuffer send_buffer(&req_slice, 1); gpr_mu_lock(&write_mu_); diff --git a/test/cpp/util/slice_test.cc b/test/cpp/util/slice_test.cc index 0f800035801..9e3329fab02 100644 --- a/test/cpp/util/slice_test.cc +++ b/test/cpp/util/slice_test.cc @@ -28,6 +28,9 @@ const char* kContent = "hello xxxxxxxxxxxxxxxxxxxx world"; class SliceTest : public ::testing::Test { protected: + void CheckSliceSize(const Slice& s, const grpc::string& content) { + EXPECT_EQ(content.size(), s.size()); + } void CheckSlice(const Slice& s, const grpc::string& content) { EXPECT_EQ(content.size(), s.size()); EXPECT_EQ(content, @@ -35,6 +38,31 @@ class SliceTest : public ::testing::Test { } }; +TEST_F(SliceTest, Empty) { + Slice empty_slice; + CheckSlice(empty_slice, ""); +} + +TEST_F(SliceTest, Sized) { + Slice sized_slice(strlen(kContent)); + CheckSliceSize(sized_slice, kContent); +} + +TEST_F(SliceTest, String) { + Slice spp(kContent); + CheckSlice(spp, kContent); +} + +TEST_F(SliceTest, Buf) { + Slice spp(kContent, strlen(kContent)); + CheckSlice(spp, kContent); +} + +TEST_F(SliceTest, StaticBuf) { + Slice spp(kContent, strlen(kContent), Slice::STATIC_SLICE); + CheckSlice(spp, kContent); +} + TEST_F(SliceTest, Steal) { grpc_slice s = grpc_slice_from_copied_string(kContent); Slice spp(s, Slice::STEAL_REF); @@ -48,11 +76,6 @@ TEST_F(SliceTest, Add) { CheckSlice(spp, kContent); } -TEST_F(SliceTest, Empty) { - Slice empty_slice; - CheckSlice(empty_slice, ""); -} - TEST_F(SliceTest, Cslice) { grpc_slice s = grpc_slice_from_copied_string(kContent); Slice spp(s, Slice::STEAL_REF); From ae5e83b664c9f05bcdf04df7aad34ed108dbf10b Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 18 Jul 2017 16:11:00 -0700 Subject: [PATCH 18/26] Free pending grpclb update args --- .../ext/filters/client_channel/lb_policy/grpclb/grpclb.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c index cccc3e871a6..fdb18f687f3 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c @@ -1705,7 +1705,6 @@ static void lb_on_server_status_received_locked(grpc_exec_ctx *exec_ctx, static void glb_update_locked(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy, const grpc_lb_policy_args *args) { glb_lb_policy *glb_policy = (glb_lb_policy *)policy; - if (glb_policy->updating_lb_channel) { if (GRPC_TRACER_ON(grpc_lb_glb_trace)) { gpr_log(GPR_INFO, @@ -1813,9 +1812,11 @@ static void glb_lb_channel_on_connectivity_changed_cb(grpc_exec_ctx *exec_ctx, // lb_on_server_status_received will pick up the cancel and reinit // lb_call. if (glb_policy->pending_update_args != NULL) { - const grpc_lb_policy_args *args = glb_policy->pending_update_args; + grpc_lb_policy_args *args = glb_policy->pending_update_args; glb_policy->pending_update_args = NULL; glb_update_locked(exec_ctx, &glb_policy->base, args); + grpc_channel_args_destroy(exec_ctx, args->args); + gpr_free(args); } } else if (glb_policy->started_picking && !glb_policy->shutting_down) { if (glb_policy->retry_timer_active) { From bbd348c5e18794b0fee65a752bc101cdde4b247d Mon Sep 17 00:00:00 2001 From: Yong Ni Date: Tue, 18 Jul 2017 08:38:11 -0700 Subject: [PATCH 19/26] Added all previous minor releases of gRPC to the matrix. Modified the testsuite name format for better display test results in sponge. --- tools/interop_matrix/client_matrix.py | 26 ++++++++++++++++--- .../run_interop_matrix_tests.py | 2 +- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index 4394e32c085..4d1b5f01695 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -30,9 +30,27 @@ LANG_RUNTIME_MATRIX = { } # Dictionary of releases per language. For each language, we need to provide -# a tuple of release tag (used as the tag for the GCR image) and also github hash. +# a release tag pointing to the latest build of the branch. LANG_RELEASE_MATRIX = { - 'cxx': ['v1.0.1', 'v1.1.2'], - 'go': ['v1.0.1-GA', 'v1.3.0'], - 'java': ['v1.0.3', 'v1.1.2'], + 'cxx': [ + 'v1.0.1', + 'v1.1.4', + 'v1.2.5', + 'v1.3.9', + 'v1.4.2', + ], + 'go': [ + 'v1.0.5', + 'v1.2.1', + 'v1.3.0', + 'v1.4.2', + ], + 'java': [ + 'v1.0.3', + 'v1.1.2', + 'v1.2.0', + 'v1.3.1', + 'v1.4.0', + 'v1.5.0', + ], } diff --git a/tools/interop_matrix/run_interop_matrix_tests.py b/tools/interop_matrix/run_interop_matrix_tests.py index 28126ddf5cc..4315c8277df 100755 --- a/tools/interop_matrix/run_interop_matrix_tests.py +++ b/tools/interop_matrix/run_interop_matrix_tests.py @@ -168,7 +168,7 @@ def run_tests_for_lang(lang, runtime, images): _xml_report_tree, resultset, 'grpc_interop_matrix', - '%s__%s:%s'%(lang,runtime,release), + '%s__%s %s'%(lang,runtime,release), str(uuid.uuid4())) _docker_images_cleanup = [] From 85fac6df5711b418fabc87680d304e25171515f8 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Wed, 12 Jul 2017 07:49:45 -0700 Subject: [PATCH 20/26] Quiet down server builder --- src/cpp/server/server_builder.cc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index c90f96c0b72..fa3b9cf7660 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -250,13 +250,6 @@ std::unique_ptr ServerBuilder::BuildAndStart() { has_sync_methods && num_frequently_polled_cqs > 0; if (has_sync_methods) { - // This is a Sync server - gpr_log(GPR_INFO, - "Synchronous server. Num CQs: %d, Min pollers: %d, Max Pollers: " - "%d, CQ timeout (msec): %d", - sync_server_settings_.num_cqs, sync_server_settings_.min_pollers, - sync_server_settings_.max_pollers, - sync_server_settings_.cq_timeout_msec); grpc_cq_polling_type polling_type = is_hybrid_server ? GRPC_CQ_NON_POLLING : GRPC_CQ_DEFAULT_POLLING; @@ -272,6 +265,16 @@ std::unique_ptr ServerBuilder::BuildAndStart() { sync_server_settings_.min_pollers, sync_server_settings_.max_pollers, sync_server_settings_.cq_timeout_msec)); + if (has_sync_methods) { + // This is a Sync server + gpr_log(GPR_INFO, + "Synchronous server. Num CQs: %d, Min pollers: %d, Max Pollers: " + "%d, CQ timeout (msec): %d", + sync_server_settings_.num_cqs, sync_server_settings_.min_pollers, + sync_server_settings_.max_pollers, + sync_server_settings_.cq_timeout_msec); + } + ServerInitializer* initializer = server->initializer(); // Register all the completion queues with the server. i.e From 79764a3bfbdec89b93b2f4a52b73e4b2ec275226 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Wed, 12 Jul 2017 07:50:09 -0700 Subject: [PATCH 21/26] Quiet down getenv --- src/core/lib/support/env.h | 2 ++ src/core/lib/support/env_linux.c | 35 +++++++++++++++++++----------- src/core/lib/support/env_posix.c | 5 +++++ src/core/lib/support/env_windows.c | 5 +++++ src/core/lib/support/log.c | 9 +++++++- 5 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/core/lib/support/env.h b/src/core/lib/support/env.h index 18bc08ac62a..392fcb6e4c0 100644 --- a/src/core/lib/support/env.h +++ b/src/core/lib/support/env.h @@ -36,6 +36,8 @@ char *gpr_getenv(const char *name); /* Sets the the environment with the specified name to the specified value. */ void gpr_setenv(const char *name, const char *value); +char *gpr_getenv_silent(const char *name, char** dst); + #ifdef __cplusplus } #endif diff --git a/src/core/lib/support/env_linux.c b/src/core/lib/support/env_linux.c index 0c79a2c4014..bc488e1b063 100644 --- a/src/core/lib/support/env_linux.c +++ b/src/core/lib/support/env_linux.c @@ -38,8 +38,10 @@ #include "src/core/lib/support/string.h" -char *gpr_getenv(const char *name) { -#if defined(GPR_BACKWARDS_COMPATIBILITY_MODE) +char *gpr_getenv_silent(const char *name, char** dst) { + char* insecure_func_used = NULL; + char* result = NULL; + #if defined(GPR_BACKWARDS_COMPATIBILITY_MODE) typedef char *(*getenv_type)(const char *); static getenv_type getenv_func = NULL; /* Check to see which getenv variant is supported (go from most @@ -48,22 +50,29 @@ char *gpr_getenv(const char *name) { for (size_t i = 0; getenv_func == NULL && i < GPR_ARRAY_SIZE(names); i++) { getenv_func = (getenv_type)dlsym(RTLD_DEFAULT, names[i]); if (getenv_func != NULL && strstr(names[i], "secure") == NULL) { - gpr_log(GPR_DEBUG, - "Warning: insecure environment read function '%s' used", - names[i]); + insecure_func_used = names[i]; } } - char *result = getenv_func(name); - return result == NULL ? result : gpr_strdup(result); + result = getenv_func(name); #elif __GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 17) - char *result = secure_getenv(name); - return result == NULL ? result : gpr_strdup(result); + result = secure_getenv(name); #else - gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used", - "getenv"); - char *result = getenv(name); - return result == NULL ? result : gpr_strdup(result); + result = getenv(name); + insecure_func_used = "getenv"; #endif + *dst = result == NULL ? result : gpr_strdup(result); + return insecure_func_used; +} + +char *gpr_getenv(const char *name) { + char* result = NULL; + char* insecure_func_used = gpr_getenv_silent(name, &result); + if (insecure_func_used != NULL) { + gpr_log(GPR_DEBUG, + "Warning: insecure environment read function '%s' used", + insecure_func_used); + } + return result; } void gpr_setenv(const char *name, const char *value) { diff --git a/src/core/lib/support/env_posix.c b/src/core/lib/support/env_posix.c index bdbc4da95a5..8cc802ef883 100644 --- a/src/core/lib/support/env_posix.c +++ b/src/core/lib/support/env_posix.c @@ -29,6 +29,11 @@ #include #include "src/core/lib/support/string.h" +char *gpr_getenv_silent(const char *name, char **dst) { + *dst = gpr_getenv(name); + return NULL; +} + char *gpr_getenv(const char *name) { char *result = getenv(name); return result == NULL ? result : gpr_strdup(result); diff --git a/src/core/lib/support/env_windows.c b/src/core/lib/support/env_windows.c index c1d557e2199..557d5d6ad50 100644 --- a/src/core/lib/support/env_windows.c +++ b/src/core/lib/support/env_windows.c @@ -30,6 +30,11 @@ #include #include +char *gpr_getenv_silent(const char *name, char **dst) { + *dst = gpr_getenv(name); + return NULL; +} + char *gpr_getenv(const char *name) { char *result = NULL; DWORD size; diff --git a/src/core/lib/support/log.c b/src/core/lib/support/log.c index bcc336b8ae7..010dc65d6ae 100644 --- a/src/core/lib/support/log.c +++ b/src/core/lib/support/log.c @@ -64,7 +64,8 @@ void gpr_set_log_verbosity(gpr_log_severity min_severity_to_print) { } void gpr_log_verbosity_init() { - char *verbosity = gpr_getenv("GRPC_VERBOSITY"); + char *verbosity = NULL; + char *insecure_getenv = gpr_getenv_silent("GRPC_VERBOSITY", &verbosity); gpr_atm min_severity_to_print = GPR_LOG_SEVERITY_ERROR; if (verbosity != NULL) { @@ -81,6 +82,12 @@ void gpr_log_verbosity_init() { GPR_LOG_VERBOSITY_UNSET) { gpr_atm_no_barrier_store(&g_min_severity_to_print, min_severity_to_print); } + + if (insecure_getenv != NULL) { + gpr_log(GPR_DEBUG, + "Warning: insecure environment read function '%s' used", + insecure_getenv); + } } void gpr_set_log_function(gpr_log_func f) { From 68d78c6330664019abaefa6e762caf50b93c989d Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 13 Jul 2017 07:25:40 -0700 Subject: [PATCH 22/26] Add comment --- src/core/lib/support/env.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/lib/support/env.h b/src/core/lib/support/env.h index 392fcb6e4c0..e1c3bdc66a3 100644 --- a/src/core/lib/support/env.h +++ b/src/core/lib/support/env.h @@ -36,6 +36,10 @@ char *gpr_getenv(const char *name); /* Sets the the environment with the specified name to the specified value. */ void gpr_setenv(const char *name, const char *value); +/* This is a version of gpr_getenv that does not produce any output if it has to + use an insecure version of the function. It is ONLY to be used to solve the + problem in which we need to check an env variable to configure the verbosity + level of logging. So DO NOT USE THIS. */ char *gpr_getenv_silent(const char *name, char** dst); #ifdef __cplusplus From b9b376ccd484b711d32dbffe748bef0f04f8bfa3 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 13 Jul 2017 07:30:15 -0700 Subject: [PATCH 23/26] Ban the silent version --- tools/run_tests/sanity/core_banned_functions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/run_tests/sanity/core_banned_functions.py b/tools/run_tests/sanity/core_banned_functions.py index b394bbbeaf1..1f139054847 100755 --- a/tools/run_tests/sanity/core_banned_functions.py +++ b/tools/run_tests/sanity/core_banned_functions.py @@ -41,6 +41,8 @@ BANNED_EXCEPT = { 'grpc_closure_sched(' : ['src/core/lib/iomgr/closure.c'], 'grpc_closure_run(' : ['src/core/lib/iomgr/closure.c'], 'grpc_closure_list_sched(' : ['src/core/lib/iomgr/closure.c'], + 'gpr_getenv_silent(' : ['src/core/lib/support/log.c', 'src/core/lib/support/env_linux.c', + 'src/core/lib/support/env_posix.c', 'src/core/lib/support/env_windows.c'], } errors = 0 From 139072ff929891ada8f98f1890d63b45ce049c64 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Thu, 13 Jul 2017 23:47:59 -0700 Subject: [PATCH 24/26] Const correct and clang --- src/core/lib/support/env.h | 2 +- src/core/lib/support/env_linux.c | 15 +++++++-------- src/core/lib/support/env_posix.c | 2 +- src/core/lib/support/env_windows.c | 2 +- src/core/lib/support/log.c | 5 ++--- src/cpp/server/server_builder.cc | 1 - 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/core/lib/support/env.h b/src/core/lib/support/env.h index e1c3bdc66a3..e2c012a7288 100644 --- a/src/core/lib/support/env.h +++ b/src/core/lib/support/env.h @@ -40,7 +40,7 @@ void gpr_setenv(const char *name, const char *value); use an insecure version of the function. It is ONLY to be used to solve the problem in which we need to check an env variable to configure the verbosity level of logging. So DO NOT USE THIS. */ -char *gpr_getenv_silent(const char *name, char** dst); +const char *gpr_getenv_silent(const char *name, char **dst); #ifdef __cplusplus } diff --git a/src/core/lib/support/env_linux.c b/src/core/lib/support/env_linux.c index bc488e1b063..4c45a977caa 100644 --- a/src/core/lib/support/env_linux.c +++ b/src/core/lib/support/env_linux.c @@ -38,10 +38,10 @@ #include "src/core/lib/support/string.h" -char *gpr_getenv_silent(const char *name, char** dst) { - char* insecure_func_used = NULL; - char* result = NULL; - #if defined(GPR_BACKWARDS_COMPATIBILITY_MODE) +const char *gpr_getenv_silent(const char *name, char **dst) { + const char *insecure_func_used = NULL; + char *result = NULL; +#if defined(GPR_BACKWARDS_COMPATIBILITY_MODE) typedef char *(*getenv_type)(const char *); static getenv_type getenv_func = NULL; /* Check to see which getenv variant is supported (go from most @@ -65,11 +65,10 @@ char *gpr_getenv_silent(const char *name, char** dst) { } char *gpr_getenv(const char *name) { - char* result = NULL; - char* insecure_func_used = gpr_getenv_silent(name, &result); + char *result = NULL; + const char *insecure_func_used = gpr_getenv_silent(name, &result); if (insecure_func_used != NULL) { - gpr_log(GPR_DEBUG, - "Warning: insecure environment read function '%s' used", + gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used", insecure_func_used); } return result; diff --git a/src/core/lib/support/env_posix.c b/src/core/lib/support/env_posix.c index 8cc802ef883..b88822ca025 100644 --- a/src/core/lib/support/env_posix.c +++ b/src/core/lib/support/env_posix.c @@ -29,7 +29,7 @@ #include #include "src/core/lib/support/string.h" -char *gpr_getenv_silent(const char *name, char **dst) { +const char *gpr_getenv_silent(const char *name, char **dst) { *dst = gpr_getenv(name); return NULL; } diff --git a/src/core/lib/support/env_windows.c b/src/core/lib/support/env_windows.c index 557d5d6ad50..652eeb61c67 100644 --- a/src/core/lib/support/env_windows.c +++ b/src/core/lib/support/env_windows.c @@ -30,7 +30,7 @@ #include #include -char *gpr_getenv_silent(const char *name, char **dst) { +const char *gpr_getenv_silent(const char *name, char **dst) { *dst = gpr_getenv(name); return NULL; } diff --git a/src/core/lib/support/log.c b/src/core/lib/support/log.c index 010dc65d6ae..fadb4d9a2cf 100644 --- a/src/core/lib/support/log.c +++ b/src/core/lib/support/log.c @@ -65,7 +65,7 @@ void gpr_set_log_verbosity(gpr_log_severity min_severity_to_print) { void gpr_log_verbosity_init() { char *verbosity = NULL; - char *insecure_getenv = gpr_getenv_silent("GRPC_VERBOSITY", &verbosity); + const char *insecure_getenv = gpr_getenv_silent("GRPC_VERBOSITY", &verbosity); gpr_atm min_severity_to_print = GPR_LOG_SEVERITY_ERROR; if (verbosity != NULL) { @@ -84,8 +84,7 @@ void gpr_log_verbosity_init() { } if (insecure_getenv != NULL) { - gpr_log(GPR_DEBUG, - "Warning: insecure environment read function '%s' used", + gpr_log(GPR_DEBUG, "Warning: insecure environment read function '%s' used", insecure_getenv); } } diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index fa3b9cf7660..200e477822c 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -250,7 +250,6 @@ std::unique_ptr ServerBuilder::BuildAndStart() { has_sync_methods && num_frequently_polled_cqs > 0; if (has_sync_methods) { - grpc_cq_polling_type polling_type = is_hybrid_server ? GRPC_CQ_NON_POLLING : GRPC_CQ_DEFAULT_POLLING; From 02d426ef6bc3c8babb4535c9ef758cee8eadce39 Mon Sep 17 00:00:00 2001 From: Ben Sykes Date: Tue, 11 Jul 2017 13:58:42 -0700 Subject: [PATCH 25/26] Check for no_proxy environment variable and skip proxy if host is listed This will fix #9989, #11603, #11751. no_proxy is typically comma separated list of hosts and or domains. Matching must be from the end of the uri to accomodate whole domain and subdomain whitelisting. This will work for well-formed no_proxy strings, but may fail if there are additional spaces. --- .../ext/filters/client_channel/http_proxy.c | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/core/ext/filters/client_channel/http_proxy.c b/src/core/ext/filters/client_channel/http_proxy.c index cfb5ec6f00e..aa3f61c9917 100644 --- a/src/core/ext/filters/client_channel/http_proxy.c +++ b/src/core/ext/filters/client_channel/http_proxy.c @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -30,6 +31,7 @@ #include "src/core/ext/filters/client_channel/uri_parser.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/support/env.h" +#include "src/core/lib/support/string.h" static char* grpc_get_http_proxy_server(grpc_exec_ctx* exec_ctx) { char* uri_str = gpr_getenv("http_proxy"); @@ -80,6 +82,50 @@ static bool proxy_mapper_map_name(grpc_exec_ctx* exec_ctx, grpc_uri_destroy(uri); return false; } + char* no_proxy_str = gpr_getenv("no_proxy"); + if (no_proxy_str != NULL) { + static const char* NO_PROXY_SEPARATOR = ","; + bool use_proxy = true; + char* server_host; + char* server_port; + if (!gpr_split_host_port(uri->path[0] == '/' ? uri->path + 1 : uri->path, + &server_host, &server_port)) { + gpr_log(GPR_INFO, + "unable to split host and port, not checking no_proxy list for " + "host '%s'", + server_uri); + } else { + size_t uri_len = strlen(server_host); + char** no_proxy_hosts; + size_t num_no_proxy_hosts; + gpr_string_split(no_proxy_str, NO_PROXY_SEPARATOR, &no_proxy_hosts, + &num_no_proxy_hosts); + for (size_t i = 0; i < num_no_proxy_hosts; i++) { + char* no_proxy_entry = no_proxy_hosts[i]; + size_t no_proxy_len = strlen(no_proxy_entry); + if (no_proxy_len <= uri_len && + gpr_stricmp(no_proxy_entry, &server_host[uri_len - no_proxy_len]) == + 0) { + gpr_log(GPR_INFO, "not using proxy for host in no_proxy list '%s'", + server_uri); + use_proxy = false; + break; + } + } + for (size_t i = 0; i < num_no_proxy_hosts; i++) { + gpr_free(no_proxy_hosts[i]); + } + gpr_free(no_proxy_hosts); + gpr_free(server_host); + gpr_free(server_port); + if (!use_proxy) { + grpc_uri_destroy(uri); + gpr_free(*name_to_resolve); + *name_to_resolve = NULL; + return false; + } + } + } grpc_arg new_arg = grpc_channel_arg_string_create( GRPC_ARG_HTTP_CONNECT_SERVER, uri->path[0] == '/' ? uri->path + 1 : uri->path); From 713d7c93a825dc21df0624be314cb2782738ff8b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 19 Jul 2017 10:19:28 +0200 Subject: [PATCH 26/26] ruby is already installed on kokoro ubuntu VMs --- tools/internal_ci/linux/grpc_build_artifacts.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/internal_ci/linux/grpc_build_artifacts.sh b/tools/internal_ci/linux/grpc_build_artifacts.sh index bc29014ab5f..3997a130878 100755 --- a/tools/internal_ci/linux/grpc_build_artifacts.sh +++ b/tools/internal_ci/linux/grpc_build_artifacts.sh @@ -20,9 +20,10 @@ cd $(dirname $0)/../../.. source tools/internal_ci/helper_scripts/prepare_build_linux_rc -# TODO(jtattermusch): install ruby on the internal_ci worker -gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 -# TODO(jtattermusch): grep works around https://github.com/rvm/rvm/issues/4068 -curl -sSL https://get.rvm.io | grep -v __rvm_print_headline | bash -s stable --ruby +set +ex +[[ -s /etc/profile.d/rvm.sh ]] && . /etc/profile.d/rvm.sh +set -e # rvm commands are very verbose +rvm --default use ruby-2.4.1 +set -ex tools/run_tests/task_runner.py -f artifact linux