From 2e0e4e91ed46c5fe39a327ab6484f4531d29788f Mon Sep 17 00:00:00 2001 From: Ken Payson Date: Tue, 22 Aug 2017 11:33:15 -0700 Subject: [PATCH 01/41] Increase Python mac target build time 30m -> 60m --- tools/run_tests/artifacts/artifact_targets.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/run_tests/artifacts/artifact_targets.py b/tools/run_tests/artifacts/artifact_targets.py index 841bbdf9120..41ab8b9339e 100644 --- a/tools/run_tests/artifacts/artifact_targets.py +++ b/tools/run_tests/artifacts/artifact_targets.py @@ -157,6 +157,7 @@ class PythonArtifact: return create_jobspec(self.name, ['tools/run_tests/artifacts/build_artifact_python.sh'], environ=environ, + timeout_seconds=60*60, use_workspace=True) def __str__(self): From f85caa63829211614a3a252bc993ea10c53c8d2e Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 22 Aug 2017 12:31:54 -0700 Subject: [PATCH 02/41] Advance dependency version of Protobuf on ObjC --- src/objective-c/!ProtoCompiler-gRPCPlugin.podspec | 2 +- src/objective-c/!ProtoCompiler.podspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec index b7387c897a7..7fc065c60c1 100644 --- a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec +++ b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec @@ -101,7 +101,7 @@ Pod::Spec.new do |s| s.preserve_paths = plugin # Restrict the protoc version to the one supported by this plugin. - s.dependency '!ProtoCompiler', '3.3.0' + s.dependency '!ProtoCompiler', '3.4.0' # For the Protobuf dependency not to complain: s.ios.deployment_target = '7.0' s.osx.deployment_target = '10.9' diff --git a/src/objective-c/!ProtoCompiler.podspec b/src/objective-c/!ProtoCompiler.podspec index c3f95f9f425..25c437911f6 100644 --- a/src/objective-c/!ProtoCompiler.podspec +++ b/src/objective-c/!ProtoCompiler.podspec @@ -36,7 +36,7 @@ Pod::Spec.new do |s| # exclamation mark ensures that other "regular" pods will be able to find it as it'll be installed # before them. s.name = '!ProtoCompiler' - v = '3.3.0' + v = '3.4.0' s.version = v s.summary = 'The Protobuf Compiler (protoc) generates Objective-C files from .proto files' s.description = <<-DESC From 9d3f3a2a39db7907a29e2e0d86b7ea242dc7d7cc Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 22 Aug 2017 17:15:58 -0700 Subject: [PATCH 03/41] fix the template --- src/objective-c/!ProtoCompiler-gRPCPlugin.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec index 7fc065c60c1..b7387c897a7 100644 --- a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec +++ b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec @@ -101,7 +101,7 @@ Pod::Spec.new do |s| s.preserve_paths = plugin # Restrict the protoc version to the one supported by this plugin. - s.dependency '!ProtoCompiler', '3.4.0' + s.dependency '!ProtoCompiler', '3.3.0' # For the Protobuf dependency not to complain: s.ios.deployment_target = '7.0' s.osx.deployment_target = '10.9' From c457b47fcf4c5e63a545a60096b9783650586773 Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 22 Aug 2017 17:19:21 -0700 Subject: [PATCH 04/41] fix the template --- src/objective-c/!ProtoCompiler-gRPCPlugin.podspec | 2 +- .../src/objective-c/!ProtoCompiler-gRPCPlugin.podspec.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec index b7387c897a7..7fc065c60c1 100644 --- a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec +++ b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec @@ -101,7 +101,7 @@ Pod::Spec.new do |s| s.preserve_paths = plugin # Restrict the protoc version to the one supported by this plugin. - s.dependency '!ProtoCompiler', '3.3.0' + s.dependency '!ProtoCompiler', '3.4.0' # For the Protobuf dependency not to complain: s.ios.deployment_target = '7.0' s.osx.deployment_target = '10.9' diff --git a/templates/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec.template b/templates/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec.template index b8223417b71..196c4054686 100644 --- a/templates/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec.template +++ b/templates/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec.template @@ -103,7 +103,7 @@ s.preserve_paths = plugin # Restrict the protoc version to the one supported by this plugin. - s.dependency '!ProtoCompiler', '3.3.0' + s.dependency '!ProtoCompiler', '3.4.0' # For the Protobuf dependency not to complain: s.ios.deployment_target = '7.0' s.osx.deployment_target = '10.9' From 2746d84282136cea8e101b392ba53a25ade863cc Mon Sep 17 00:00:00 2001 From: Muxi Yan Date: Tue, 22 Aug 2017 18:19:14 -0700 Subject: [PATCH 05/41] Add missing files to podspec --- gRPC-Core.podspec | 1 + templates/gRPC-Core.podspec.template | 1 + 2 files changed, 2 insertions(+) diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 77ddc6a33e2..083305762a1 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -987,6 +987,7 @@ Pod::Spec.new do |s| 'test/core/end2end/end2end_tests.{c,h}', 'test/core/end2end/end2end_test_utils.c', 'test/core/end2end/tests/*.{c,h}', + 'test/core/end2end/fixtures/*.h', 'test/core/end2end/data/*.{c,h}', 'test/core/util/debugger_macros.{c,h}', 'test/core/util/test_config.{c,h}', diff --git a/templates/gRPC-Core.podspec.template b/templates/gRPC-Core.podspec.template index 616f2cbbff9..6077f8098dd 100644 --- a/templates/gRPC-Core.podspec.template +++ b/templates/gRPC-Core.podspec.template @@ -171,6 +171,7 @@ 'test/core/end2end/end2end_tests.{c,h}', 'test/core/end2end/end2end_test_utils.c', 'test/core/end2end/tests/*.{c,h}', + 'test/core/end2end/fixtures/*.h', 'test/core/end2end/data/*.{c,h}', 'test/core/util/debugger_macros.{c,h}', 'test/core/util/test_config.{c,h}', From 66999d219274b278c02ffcd3e40c0902e904c6c7 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 23 Aug 2017 13:51:42 -0700 Subject: [PATCH 06/41] fix build.yaml to expand nanopb headers file group --- build.yaml | 2 +- grpc.gemspec | 4 ++++ package.xml | 4 ++++ tools/doxygen/Doxyfile.core.internal | 6 +++++- vsprojects/vcxproj/grpc/grpc.vcxproj | 4 ++++ vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 12 ++++++++++++ .../vcxproj/grpc_unsecure/grpc_unsecure.vcxproj | 4 ++++ .../grpc_unsecure/grpc_unsecure.vcxproj.filters | 12 ++++++++++++ 8 files changed, 46 insertions(+), 2 deletions(-) diff --git a/build.yaml b/build.yaml index dba7da12450..905d0e4cd9d 100644 --- a/build.yaml +++ b/build.yaml @@ -911,7 +911,7 @@ filegroups: - third_party/nanopb/pb_common.c - third_party/nanopb/pb_decode.c - third_party/nanopb/pb_encode.c - filegroups: + uses: - nanopb_headers - name: nanopb_headers headers: diff --git a/grpc.gemspec b/grpc.gemspec index f04a14167b1..9957f050d0c 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -369,6 +369,10 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb_client_stats.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/load_balancer_api.h ) s.files += %w( src/core/ext/filters/client_channel/lb_policy/grpclb/proto/grpc/lb/v1/load_balancer.pb.h ) + s.files += %w( third_party/nanopb/pb.h ) + s.files += %w( third_party/nanopb/pb_common.h ) + s.files += %w( third_party/nanopb/pb_decode.h ) + s.files += %w( third_party/nanopb/pb_encode.h ) s.files += %w( src/core/ext/filters/client_channel/resolver/fake/fake_resolver.h ) s.files += %w( src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_ev_driver.h ) s.files += %w( src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.h ) diff --git a/package.xml b/package.xml index e21fa6212ca..c970a918d2f 100644 --- a/package.xml +++ b/package.xml @@ -383,6 +383,10 @@ + + + + diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index ea0ebf1704f..771e7fc66bb 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1424,9 +1424,13 @@ src/core/tsi/transport_security_adapter.h \ src/core/tsi/transport_security_grpc.c \ src/core/tsi/transport_security_grpc.h \ src/core/tsi/transport_security_interface.h \ +third_party/nanopb/pb.h \ third_party/nanopb/pb_common.c \ +third_party/nanopb/pb_common.h \ third_party/nanopb/pb_decode.c \ -third_party/nanopb/pb_encode.c +third_party/nanopb/pb_decode.h \ +third_party/nanopb/pb_encode.c \ +third_party/nanopb/pb_encode.h # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 8a659280a46..33e3901e382 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -494,6 +494,10 @@ + + + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 6879047f55b..d5ef8221727 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -1454,6 +1454,18 @@ src\core\ext\filters\client_channel\lb_policy\grpclb\proto\grpc\lb\v1 + + third_party\nanopb + + + third_party\nanopb + + + third_party\nanopb + + + third_party\nanopb + src\core\ext\filters\client_channel\resolver\fake diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 945ca2f3d72..38e8a3a52a4 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -462,6 +462,10 @@ + + + + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index deb3433168d..cceb79936c6 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -1292,6 +1292,18 @@ src\core\ext\filters\client_channel\lb_policy\grpclb\proto\grpc\lb\v1 + + third_party\nanopb + + + third_party\nanopb + + + third_party\nanopb + + + third_party\nanopb + src\core\ext\census From f5f5ed0305d1d4d59b85f9a00786a876be098f1a Mon Sep 17 00:00:00 2001 From: Ken Payson Date: Thu, 24 Aug 2017 11:46:03 -0700 Subject: [PATCH 07/41] Fix syntax error on classifiers --- setup.py | 2 +- src/python/grpcio_health_checking/setup.py | 2 +- src/python/grpcio_reflection/setup.py | 2 +- tools/distrib/python/grpcio_tools/setup.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index 403c13562cc..c90ca8652a1 100644 --- a/setup.py +++ b/setup.py @@ -70,7 +70,7 @@ CLASSIFIERS = [ 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'License :: OSI Approved :: Apache Software License', -], +] # Environment variable to determine whether or not the Cython extension should # *use* Cython or use the generated C files. Note that this requires the C files diff --git a/src/python/grpcio_health_checking/setup.py b/src/python/grpcio_health_checking/setup.py index 0299b4cca95..1f5e9c5130b 100644 --- a/src/python/grpcio_health_checking/setup.py +++ b/src/python/grpcio_health_checking/setup.py @@ -34,7 +34,7 @@ CLASSIFIERS = [ 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'License :: OSI Approved :: Apache Software License', -], +] PACKAGE_DIRECTORIES = { '': '.', diff --git a/src/python/grpcio_reflection/setup.py b/src/python/grpcio_reflection/setup.py index bed2311b598..9360550afbc 100644 --- a/src/python/grpcio_reflection/setup.py +++ b/src/python/grpcio_reflection/setup.py @@ -35,7 +35,7 @@ CLASSIFIERS = [ 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'License :: OSI Approved :: Apache Software License', -], +] PACKAGE_DIRECTORIES = { '': '.', diff --git a/tools/distrib/python/grpcio_tools/setup.py b/tools/distrib/python/grpcio_tools/setup.py index 5c0329bff0c..e0e9226211d 100644 --- a/tools/distrib/python/grpcio_tools/setup.py +++ b/tools/distrib/python/grpcio_tools/setup.py @@ -47,7 +47,7 @@ CLASSIFIERS = [ 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'License :: OSI Approved :: Apache Software License', -], +] PY3 = sys.version_info.major == 3 From abbeefa8a462530b29f2341bd798c6f3f47f1350 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 24 Aug 2017 12:28:14 -0700 Subject: [PATCH 08/41] PHP: backport master features into v1.6.x branch --- package.xml | 12 +++---- src/php/ext/grpc/call_credentials.c | 2 +- src/php/ext/grpc/channel.c | 40 +++++++++++----------- src/php/ext/grpc/channel_credentials.c | 8 +++-- src/php/ext/grpc/php_grpc.c | 42 +++++++++++++++++++----- src/php/ext/grpc/php_grpc.h | 8 +++-- src/php/lib/Grpc/ServerStreamingCall.php | 28 ++++++++++++---- src/php/lib/Grpc/UnaryCall.php | 28 ++++++++++++---- src/php/tests/unit_tests/ChannelTest.php | 15 +++++---- templates/package.xml.template | 12 +++---- 10 files changed, 125 insertions(+), 70 deletions(-) diff --git a/package.xml b/package.xml index c970a918d2f..ce678492735 100644 --- a/package.xml +++ b/package.xml @@ -10,7 +10,7 @@ grpc-packages@google.com yes - 2017-05-22 + 2017-08-24 1.6.0RC1 @@ -22,13 +22,9 @@ Apache 2.0 -- Fixed some memory leaks #9559, #10996 -- Disabled cares dependency from gRPC C Core #10940 -- De-coupled protobuf dependency #11112 -- Fixed extension reported version #10842 -- Added config.w32 for Windows support #8161 -- Fixed PHP distrib test after cc files were added #11193 -- Fixed protoc plugin comment escape bug #11025 +- License changed to Apache 2.0 +- Added support for php_namespace option in codegen plugin #11886 +- Updated gRPC C Core library version 1.6 diff --git a/src/php/ext/grpc/call_credentials.c b/src/php/ext/grpc/call_credentials.c index f46091d7098..1eee8645df9 100644 --- a/src/php/ext/grpc/call_credentials.c +++ b/src/php/ext/grpc/call_credentials.c @@ -179,7 +179,7 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context, grpc_metadata_array metadata; bool cleanup = true; - if (Z_TYPE_P(retval) != IS_ARRAY) { + if (retval == NULL || Z_TYPE_P(retval) != IS_ARRAY) { cleanup = false; code = GRPC_STATUS_INVALID_ARGUMENT; } else if (!create_metadata_array(retval, &metadata)) { diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c index f1187e8722f..dc3acc89bb7 100644 --- a/src/php/ext/grpc/channel.c +++ b/src/php/ext/grpc/channel.c @@ -66,6 +66,8 @@ PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel) if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), p->wrapper->key, key_len, rsrc))) { grpc_channel_destroy(p->wrapper->wrapped); + free(p->wrapper->target); + free(p->wrapper->args_hashstr); } gpr_mu_unlock(&global_persistent_list_mu); } @@ -158,7 +160,7 @@ void create_and_add_channel_to_persistent_list( grpc_channel_args args, wrapped_grpc_channel_credentials *creds, char *key, - php_grpc_int key_len) { + php_grpc_int key_len TSRMLS_DC) { php_grpc_zend_resource new_rsrc; channel_persistent_le_t *le; // this links each persistent list entry to a destructor @@ -187,10 +189,8 @@ void create_and_add_channel_to_persistent_list( * credentials. * * If the $args array contains a "force_new" key mapping to a boolean value - * of "true", a new underlying grpc_channel will be created regardless. If - * there are any opened channels on the same hostname, user must manually - * call close() on those dangling channels before the end of the PHP - * script. + * of "true", a new and separate underlying grpc_channel will be created + * and returned. This will not affect existing channels. * * @param string $target The hostname to associate with this channel * @param array $args_array The arguments to pass to the Channel @@ -273,19 +273,15 @@ PHP_METHOD(Channel, __construct) { } channel->wrapper = malloc(sizeof(grpc_channel_wrapper)); channel->wrapper->key = key; - channel->wrapper->target = target; - channel->wrapper->args_hashstr = sha1str; + channel->wrapper->target = strdup(target); + channel->wrapper->args_hashstr = strdup(sha1str); if (creds != NULL && creds->hashstr != NULL) { channel->wrapper->creds_hashstr = creds->hashstr; } gpr_mu_init(&channel->wrapper->mu); smart_str_free(&buf); - if (force_new) { - php_grpc_delete_persistent_list_entry(key, key_len TSRMLS_CC); - } - - if (creds != NULL && creds->has_call_creds) { + if (force_new || (creds != NULL && creds->has_call_creds)) { // If the ChannelCredentials object was composed with a CallCredentials // object, there is no way we can tell them apart. Do NOT persist // them. They should be individually destroyed. @@ -293,7 +289,7 @@ PHP_METHOD(Channel, __construct) { } else if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), key, key_len, rsrc))) { create_and_add_channel_to_persistent_list( - channel, target, args, creds, key, key_len); + channel, target, args, creds, key, key_len TSRMLS_CC); } else { // Found a previously stored channel in the persistent list channel_persistent_le_t *le = (channel_persistent_le_t *)rsrc->ptr; @@ -303,7 +299,7 @@ PHP_METHOD(Channel, __construct) { strcmp(creds->hashstr, le->channel->creds_hashstr) != 0)) { // somehow hash collision create_and_add_channel_to_persistent_list( - channel, target, args, creds, key, key_len); + channel, target, args, creds, key, key_len TSRMLS_CC); } else { channel->wrapper = le->channel; } @@ -416,12 +412,14 @@ PHP_METHOD(Channel, close) { gpr_mu_lock(&channel->wrapper->mu); if (channel->wrapper->wrapped != NULL) { grpc_channel_destroy(channel->wrapper->wrapped); + free(channel->wrapper->target); + free(channel->wrapper->args_hashstr); channel->wrapper->wrapped = NULL; - } - php_grpc_delete_persistent_list_entry(channel->wrapper->key, - strlen(channel->wrapper->key) - TSRMLS_CC); + php_grpc_delete_persistent_list_entry(channel->wrapper->key, + strlen(channel->wrapper->key) + TSRMLS_CC); + } gpr_mu_unlock(&channel->wrapper->mu); } @@ -449,12 +447,11 @@ static void php_grpc_channel_plink_dtor(php_grpc_zend_resource *rsrc gpr_mu_lock(&le->channel->mu); if (le->channel->wrapped != NULL) { grpc_channel_destroy(le->channel->wrapped); - free(le->channel->key); - free(le->channel); + free(le->channel->target); + free(le->channel->args_hashstr); } gpr_mu_unlock(&le->channel->mu); } - free(le); } ZEND_BEGIN_ARG_INFO_EX(arginfo_construct, 0, 0, 2) @@ -496,6 +493,7 @@ GRPC_STARTUP_FUNCTION(channel) { INIT_CLASS_ENTRY(ce, "Grpc\\Channel", channel_methods); ce.create_object = create_wrapped_grpc_channel; grpc_ce_channel = zend_register_internal_class(&ce TSRMLS_CC); + gpr_mu_init(&global_persistent_list_mu); le_plink = zend_register_list_destructors_ex( NULL, php_grpc_channel_plink_dtor, "Persistent Channel", module_number); PHP_GRPC_INIT_HANDLER(wrapped_grpc_channel, channel_ce_handlers); diff --git a/src/php/ext/grpc/channel_credentials.c b/src/php/ext/grpc/channel_credentials.c index 19e1cefb6f5..86e4f46c67a 100644 --- a/src/php/ext/grpc/channel_credentials.c +++ b/src/php/ext/grpc/channel_credentials.c @@ -135,6 +135,8 @@ PHP_METHOD(ChannelCredentials, createSsl) { pem_key_cert_pair.private_key = pem_key_cert_pair.cert_chain = NULL; + grpc_set_ssl_roots_override_callback(get_ssl_roots_override); + /* "|s!s!s!" == 3 optional nullable strings */ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|s!s!s!", &pem_root_certs, &root_certs_length, @@ -148,7 +150,7 @@ PHP_METHOD(ChannelCredentials, createSsl) { } php_grpc_int hashkey_len = root_certs_length + cert_chain_length; - char hashkey[hashkey_len]; + char *hashkey = emalloc(hashkey_len); if (root_certs_length > 0) { strcpy(hashkey, pem_root_certs); } @@ -164,6 +166,7 @@ PHP_METHOD(ChannelCredentials, createSsl) { pem_key_cert_pair.private_key == NULL ? NULL : &pem_key_cert_pair, NULL); zval *creds_object = grpc_php_wrap_channel_credentials(creds, hashstr, false TSRMLS_CC); + efree(hashkey); RETURN_DESTROY_ZVAL(creds_object); } @@ -177,6 +180,8 @@ PHP_METHOD(ChannelCredentials, createComposite) { zval *cred1_obj; zval *cred2_obj; + grpc_set_ssl_roots_override_callback(get_ssl_roots_override); + /* "OO" == 2 Objects */ if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "OO", &cred1_obj, grpc_ce_channel_credentials, &cred2_obj, @@ -245,7 +250,6 @@ void grpc_init_channel_credentials(TSRMLS_D) { zend_class_entry ce; INIT_CLASS_ENTRY(ce, "Grpc\\ChannelCredentials", channel_credentials_methods); - grpc_set_ssl_roots_override_callback(get_ssl_roots_override); ce.create_object = create_wrapped_grpc_channel_credentials; grpc_ce_channel_credentials = zend_register_internal_class(&ce TSRMLS_CC); PHP_GRPC_INIT_HANDLER(wrapped_grpc_channel_credentials, diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c index a96daf7d9b8..4ed56de9930 100644 --- a/src/php/ext/grpc/php_grpc.c +++ b/src/php/ext/grpc/php_grpc.c @@ -34,7 +34,8 @@ #include #include "php_grpc.h" -// ZEND_DECLARE_MODULE_GLOBALS(grpc) +ZEND_DECLARE_MODULE_GLOBALS(grpc) +static PHP_GINIT_FUNCTION(grpc); /* {{{ grpc_functions[] * @@ -55,13 +56,17 @@ zend_module_entry grpc_module_entry = { grpc_functions, PHP_MINIT(grpc), PHP_MSHUTDOWN(grpc), - NULL, + PHP_RINIT(grpc), NULL, PHP_MINFO(grpc), #if ZEND_MODULE_API_NO >= 20010901 PHP_GRPC_VERSION, #endif - STANDARD_MODULE_PROPERTIES}; + PHP_MODULE_GLOBALS(grpc), + PHP_GINIT(grpc), + NULL, + NULL, + STANDARD_MODULE_PROPERTIES_EX}; /* }}} */ #ifdef COMPILE_DL_GRPC @@ -99,7 +104,6 @@ PHP_MINIT_FUNCTION(grpc) { REGISTER_INI_ENTRIES(); */ /* Register call error constants */ - grpc_init(); REGISTER_LONG_CONSTANT("Grpc\\CALL_OK", GRPC_CALL_OK, CONST_CS | CONST_PERSISTENT); REGISTER_LONG_CONSTANT("Grpc\\CALL_ERROR", GRPC_CALL_ERROR, @@ -227,7 +231,6 @@ PHP_MINIT_FUNCTION(grpc) { grpc_init_channel_credentials(TSRMLS_C); grpc_init_call_credentials(TSRMLS_C); grpc_init_server_credentials(TSRMLS_C); - grpc_php_init_completion_queue(TSRMLS_C); return SUCCESS; } /* }}} */ @@ -240,9 +243,12 @@ PHP_MSHUTDOWN_FUNCTION(grpc) { */ // WARNING: This function IS being called by PHP when the extension // is unloaded but the logs were somehow suppressed. - grpc_shutdown_timeval(TSRMLS_C); - grpc_php_shutdown_completion_queue(TSRMLS_C); - grpc_shutdown(); + if (GRPC_G(initialized)) { + grpc_shutdown_timeval(TSRMLS_C); + grpc_php_shutdown_completion_queue(TSRMLS_C); + grpc_shutdown(); + GRPC_G(initialized) = 0; + } return SUCCESS; } /* }}} */ @@ -259,6 +265,26 @@ PHP_MINFO_FUNCTION(grpc) { */ } /* }}} */ + +/* {{{ PHP_RINIT_FUNCTION + */ +PHP_RINIT_FUNCTION(grpc) { + if (!GRPC_G(initialized)) { + grpc_init(); + grpc_php_init_completion_queue(TSRMLS_C); + GRPC_G(initialized) = 1; + } + return SUCCESS; +} +/* }}} */ + +/* {{{ PHP_GINIT_FUNCTION + */ +static PHP_GINIT_FUNCTION(grpc) { + grpc_globals->initialized = 0; +} +/* }}} */ + /* The previous line is meant for vim and emacs, so it can correctly fold and unfold functions in source code. See the corresponding marks just before function definition, where the functions purpose is also documented. Please diff --git a/src/php/ext/grpc/php_grpc.h b/src/php/ext/grpc/php_grpc.h index 32329178dc4..e30f011c395 100644 --- a/src/php/ext/grpc/php_grpc.h +++ b/src/php/ext/grpc/php_grpc.h @@ -49,14 +49,16 @@ PHP_MINIT_FUNCTION(grpc); PHP_MSHUTDOWN_FUNCTION(grpc); /* Displays information about the module */ PHP_MINFO_FUNCTION(grpc); +/* Code that runs at request start */ +PHP_RINIT_FUNCTION(grpc); /* Declare any global variables you may need between the BEGIN and END macros here: - -ZEND_BEGIN_MODULE_GLOBALS(grpc) -ZEND_END_MODULE_GLOBALS(grpc) */ +ZEND_BEGIN_MODULE_GLOBALS(grpc) + zend_bool initialized; +ZEND_END_MODULE_GLOBALS(grpc) /* In every utility function you add that needs to use variables in php_grpc_globals, call TSRMLS_FETCH(); after declaring other diff --git a/src/php/lib/Grpc/ServerStreamingCall.php b/src/php/lib/Grpc/ServerStreamingCall.php index 269a99ab593..f8fddfea01c 100644 --- a/src/php/lib/Grpc/ServerStreamingCall.php +++ b/src/php/lib/Grpc/ServerStreamingCall.php @@ -40,13 +40,11 @@ class ServerStreamingCall extends AbstractCall if (array_key_exists('flags', $options)) { $message_array['flags'] = $options['flags']; } - $event = $this->call->startBatch([ + $this->call->startBatch([ OP_SEND_INITIAL_METADATA => $metadata, - OP_RECV_INITIAL_METADATA => true, OP_SEND_MESSAGE => $message_array, OP_SEND_CLOSE_FROM_CLIENT => true, ]); - $this->metadata = $event->metadata; } /** @@ -54,9 +52,15 @@ class ServerStreamingCall extends AbstractCall */ public function responses() { - $response = $this->call->startBatch([ - OP_RECV_MESSAGE => true, - ])->message; + $batch = [OP_RECV_MESSAGE => true]; + if ($this->metadata === null) { + $batch[OP_RECV_INITIAL_METADATA] = true; + } + $read_event = $this->call->startBatch($batch); + if ($this->metadata === null) { + $this->metadata = $read_event->metadata; + } + $response = $read_event->message; while ($response !== null) { yield $this->_deserializeResponse($response); $response = $this->call->startBatch([ @@ -81,4 +85,16 @@ class ServerStreamingCall extends AbstractCall return $status_event->status; } + + /** + * @return mixed The metadata sent by the server + */ + public function getMetadata() + { + if ($this->metadata === null) { + $event = $this->call->startBatch([OP_RECV_INITIAL_METADATA => true]); + $this->metadata = $event->metadata; + } + return $this->metadata; + } } diff --git a/src/php/lib/Grpc/UnaryCall.php b/src/php/lib/Grpc/UnaryCall.php index dd55d02165e..e8496492b86 100644 --- a/src/php/lib/Grpc/UnaryCall.php +++ b/src/php/lib/Grpc/UnaryCall.php @@ -40,13 +40,11 @@ class UnaryCall extends AbstractCall if (isset($options['flags'])) { $message_array['flags'] = $options['flags']; } - $event = $this->call->startBatch([ + $this->call->startBatch([ OP_SEND_INITIAL_METADATA => $metadata, - OP_RECV_INITIAL_METADATA => true, OP_SEND_MESSAGE => $message_array, OP_SEND_CLOSE_FROM_CLIENT => true, ]); - $this->metadata = $event->metadata; } /** @@ -56,14 +54,32 @@ class UnaryCall extends AbstractCall */ public function wait() { - $event = $this->call->startBatch([ + $batch = [ OP_RECV_MESSAGE => true, OP_RECV_STATUS_ON_CLIENT => true, - ]); - + ]; + if ($this->metadata === null) { + $batch[OP_RECV_INITIAL_METADATA] = true; + } + $event = $this->call->startBatch($batch); + if ($this->metadata === null) { + $this->metadata = $event->metadata; + } $status = $event->status; $this->trailing_metadata = $status->metadata; return [$this->_deserializeResponse($event->message), $status]; } + + /** + * @return mixed The metadata sent by the server + */ + public function getMetadata() + { + if ($this->metadata === null) { + $event = $this->call->startBatch([OP_RECV_INITIAL_METADATA => true]); + $this->metadata = $event->metadata; + } + return $this->metadata; + } } diff --git a/src/php/tests/unit_tests/ChannelTest.php b/src/php/tests/unit_tests/ChannelTest.php index 400df0fb66d..c375a16269d 100644 --- a/src/php/tests/unit_tests/ChannelTest.php +++ b/src/php/tests/unit_tests/ChannelTest.php @@ -517,8 +517,6 @@ class ChannelTest extends PHPUnit_Framework_TestCase $state = $this->channel2->getConnectivityState(); $this->assertEquals(GRPC\CHANNEL_IDLE, $state); - // any dangling old connection to the same host must be - // manually closed $this->channel1->close(); $this->channel2->close(); } @@ -529,6 +527,7 @@ class ChannelTest extends PHPUnit_Framework_TestCase $this->channel1 = new Grpc\Channel('localhost:1', []); $this->channel2 = new Grpc\Channel('localhost:1', ["force_new" => true]); + // channel3 shares with channel1 $this->channel3 = new Grpc\Channel('localhost:1', []); // try to connect on channel2 @@ -540,29 +539,31 @@ class ChannelTest extends PHPUnit_Framework_TestCase $state = $this->channel2->getConnectivityState(); $this->assertConnecting($state); $state = $this->channel3->getConnectivityState(); - $this->assertConnecting($state); + $this->assertEquals(GRPC\CHANNEL_IDLE, $state); $this->channel1->close(); $this->channel2->close(); } + /** + * @expectedException RuntimeException + */ public function testPersistentChannelForceNewOldChannelClose() { $this->channel1 = new Grpc\Channel('localhost:1', []); $this->channel2 = new Grpc\Channel('localhost:1', ["force_new" => true]); + // channel3 shares with channel1 $this->channel3 = new Grpc\Channel('localhost:1', []); $this->channel1->close(); $state = $this->channel2->getConnectivityState(); $this->assertEquals(GRPC\CHANNEL_IDLE, $state); - $state = $this->channel3->getConnectivityState(); - $this->assertEquals(GRPC\CHANNEL_IDLE, $state); - $this->channel2->close(); - $this->channel3->close(); + // channel3 already closed + $state = $this->channel3->getConnectivityState(); } public function testPersistentChannelForceNewNewChannelClose() diff --git a/templates/package.xml.template b/templates/package.xml.template index 27cc4d01a99..a378b58e0cb 100644 --- a/templates/package.xml.template +++ b/templates/package.xml.template @@ -12,7 +12,7 @@ grpc-packages@google.com yes - 2017-05-22 + 2017-08-24 ${settings.php_version.php()} @@ -24,13 +24,9 @@ Apache 2.0 - - Fixed some memory leaks #9559, #10996 - - Disabled cares dependency from gRPC C Core #10940 - - De-coupled protobuf dependency #11112 - - Fixed extension reported version #10842 - - Added config.w32 for Windows support #8161 - - Fixed PHP distrib test after cc files were added #11193 - - Fixed protoc plugin comment escape bug #11025 + - License changed to Apache 2.0 + - Added support for php_namespace option in codegen plugin #11886 + - Updated gRPC C Core library version 1.6 From dfde2881487f05ec4e528c40510029833ed5c8c9 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Tue, 29 Aug 2017 11:31:50 -0700 Subject: [PATCH 09/41] 1.6.0-pre1 -> 1.6.0 --- BUILD | 2 +- CMakeLists.txt | 2 +- Makefile | 4 ++-- build.yaml | 2 +- gRPC-Core.podspec | 2 +- gRPC-ProtoRPC.podspec | 2 +- gRPC-RxLibrary.podspec | 2 +- gRPC.podspec | 2 +- package.json | 2 +- package.xml | 4 ++-- src/cpp/common/version_cc.cc | 2 +- src/csharp/Grpc.Core/Version.csproj.include | 2 +- src/csharp/Grpc.Core/VersionInfo.cs | 2 +- src/csharp/build_packages_dotnetcli.bat | 2 +- src/csharp/build_packages_dotnetcli.sh | 4 ++-- src/node/health_check/package.json | 4 ++-- src/node/tools/package.json | 2 +- src/objective-c/!ProtoCompiler-gRPCPlugin.podspec | 2 +- src/objective-c/GRPCClient/private/version.h | 2 +- src/php/ext/grpc/version.h | 2 +- src/python/grpcio/grpc/_grpcio_metadata.py | 2 +- src/python/grpcio/grpc_version.py | 2 +- src/python/grpcio_health_checking/grpc_version.py | 2 +- src/python/grpcio_reflection/grpc_version.py | 2 +- src/python/grpcio_tests/grpc_version.py | 2 +- src/ruby/lib/grpc/version.rb | 2 +- src/ruby/tools/version.rb | 2 +- tools/distrib/python/grpcio_tools/grpc_version.py | 2 +- tools/doxygen/Doxyfile.c++ | 2 +- tools/doxygen/Doxyfile.c++.internal | 2 +- 30 files changed, 34 insertions(+), 34 deletions(-) diff --git a/BUILD b/BUILD index 26a8af9fc26..4d1121a79d5 100644 --- a/BUILD +++ b/BUILD @@ -38,7 +38,7 @@ g_stands_for = "garcia" core_version = "4.0.0" -version = "1.6.0-pre1" +version = "1.6.0" GPR_PUBLIC_HDRS = [ "include/grpc/support/alloc.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 8fea5079870..ebbaa6a9ec7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,7 @@ cmake_minimum_required(VERSION 2.8) set(PACKAGE_NAME "grpc") -set(PACKAGE_VERSION "1.6.0-pre1") +set(PACKAGE_VERSION "1.6.0") set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}") set(PACKAGE_TARNAME "${PACKAGE_NAME}-${PACKAGE_VERSION}") set(PACKAGE_BUGREPORT "https://github.com/grpc/grpc/issues/") diff --git a/Makefile b/Makefile index b47fd222d57..801f8a7d6ed 100644 --- a/Makefile +++ b/Makefile @@ -411,8 +411,8 @@ Q = @ endif CORE_VERSION = 4.0.0 -CPP_VERSION = 1.6.0-pre1 -CSHARP_VERSION = 1.6.0-pre1 +CPP_VERSION = 1.6.0 +CSHARP_VERSION = 1.6.0 CPPFLAGS_NO_ARCH += $(addprefix -I, $(INCLUDES)) $(addprefix -D, $(DEFINES)) CPPFLAGS += $(CPPFLAGS_NO_ARCH) $(ARCH_FLAGS) diff --git a/build.yaml b/build.yaml index 905d0e4cd9d..723c8fe4460 100644 --- a/build.yaml +++ b/build.yaml @@ -14,7 +14,7 @@ settings: '#10': See the expand_version.py for all the quirks here core_version: 4.0.0 g_stands_for: garcia - version: 1.6.0-pre1 + version: 1.6.0 filegroups: - name: census public_headers: diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 083305762a1..222d3e4f0f5 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -22,7 +22,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-Core' - version = '1.6.0-pre1' + version = '1.6.0' s.version = version s.summary = 'Core cross-platform gRPC library, written in C' s.homepage = 'https://grpc.io' diff --git a/gRPC-ProtoRPC.podspec b/gRPC-ProtoRPC.podspec index e15d4d9bd7f..2d2f31d3239 100644 --- a/gRPC-ProtoRPC.podspec +++ b/gRPC-ProtoRPC.podspec @@ -21,7 +21,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-ProtoRPC' - version = '1.6.0-pre1' + version = '1.6.0' s.version = version s.summary = 'RPC library for Protocol Buffers, based on gRPC' s.homepage = 'https://grpc.io' diff --git a/gRPC-RxLibrary.podspec b/gRPC-RxLibrary.podspec index 50c3f36d62b..38a159404fd 100644 --- a/gRPC-RxLibrary.podspec +++ b/gRPC-RxLibrary.podspec @@ -21,7 +21,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-RxLibrary' - version = '1.6.0-pre1' + version = '1.6.0' s.version = version s.summary = 'Reactive Extensions library for iOS/OSX.' s.homepage = 'https://grpc.io' diff --git a/gRPC.podspec b/gRPC.podspec index b8a4d92f054..12591cda336 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -20,7 +20,7 @@ Pod::Spec.new do |s| s.name = 'gRPC' - version = '1.6.0-pre1' + version = '1.6.0' s.version = version s.summary = 'gRPC client library for iOS/OSX' s.homepage = 'https://grpc.io' diff --git a/package.json b/package.json index 4dd51237ce1..94e6391a8d8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "grpc", - "version": "1.6.0-pre1", + "version": "1.6.0", "author": "Google Inc.", "description": "gRPC Library for Node", "homepage": "https://grpc.io/", diff --git a/package.xml b/package.xml index ce678492735..30018620dd7 100644 --- a/package.xml +++ b/package.xml @@ -13,8 +13,8 @@ 2017-08-24 - 1.6.0RC1 - 1.6.0RC1 + 1.6.0 + 1.6.0 beta diff --git a/src/cpp/common/version_cc.cc b/src/cpp/common/version_cc.cc index de303036654..c65185c77ee 100644 --- a/src/cpp/common/version_cc.cc +++ b/src/cpp/common/version_cc.cc @@ -22,5 +22,5 @@ #include namespace grpc { -grpc::string Version() { return "1.6.0-pre1"; } +grpc::string Version() { return "1.6.0"; } } diff --git a/src/csharp/Grpc.Core/Version.csproj.include b/src/csharp/Grpc.Core/Version.csproj.include index 178b0369535..23f6e409091 100755 --- a/src/csharp/Grpc.Core/Version.csproj.include +++ b/src/csharp/Grpc.Core/Version.csproj.include @@ -1,7 +1,7 @@ - 1.6.0-pre1 + 1.6.0 3.3.0 diff --git a/src/csharp/Grpc.Core/VersionInfo.cs b/src/csharp/Grpc.Core/VersionInfo.cs index 619955423ba..d402c323c1f 100644 --- a/src/csharp/Grpc.Core/VersionInfo.cs +++ b/src/csharp/Grpc.Core/VersionInfo.cs @@ -38,6 +38,6 @@ namespace Grpc.Core /// /// Current version of gRPC C# /// - public const string CurrentVersion = "1.6.0-pre1"; + public const string CurrentVersion = "1.6.0"; } } diff --git a/src/csharp/build_packages_dotnetcli.bat b/src/csharp/build_packages_dotnetcli.bat index f47f8daa4b8..b7aa22b2928 100755 --- a/src/csharp/build_packages_dotnetcli.bat +++ b/src/csharp/build_packages_dotnetcli.bat @@ -13,7 +13,7 @@ @rem limitations under the License. @rem Current package versions -set VERSION=1.6.0-pre1 +set VERSION=1.6.0 @rem Adjust the location of nuget.exe set NUGET=C:\nuget\nuget.exe diff --git a/src/csharp/build_packages_dotnetcli.sh b/src/csharp/build_packages_dotnetcli.sh index 125b1dfdd2e..455b930525b 100755 --- a/src/csharp/build_packages_dotnetcli.sh +++ b/src/csharp/build_packages_dotnetcli.sh @@ -39,7 +39,7 @@ dotnet pack --configuration Release Grpc.Auth --output ../../../artifacts dotnet pack --configuration Release Grpc.HealthCheck --output ../../../artifacts dotnet pack --configuration Release Grpc.Reflection --output ../../../artifacts -nuget pack Grpc.nuspec -Version "1.6.0-pre1" -OutputDirectory ../../artifacts -nuget pack Grpc.Tools.nuspec -Version "1.6.0-pre1" -OutputDirectory ../../artifacts +nuget pack Grpc.nuspec -Version "1.6.0" -OutputDirectory ../../artifacts +nuget pack Grpc.Tools.nuspec -Version "1.6.0" -OutputDirectory ../../artifacts (cd ../../artifacts && zip csharp_nugets_dotnetcli.zip *.nupkg) diff --git a/src/node/health_check/package.json b/src/node/health_check/package.json index 30e6df6b381..66ab2a292fe 100644 --- a/src/node/health_check/package.json +++ b/src/node/health_check/package.json @@ -1,6 +1,6 @@ { "name": "grpc-health-check", - "version": "1.6.0-pre1", + "version": "1.6.0", "author": "Google Inc.", "description": "Health check service for use with gRPC", "repository": { @@ -15,7 +15,7 @@ } ], "dependencies": { - "grpc": "^1.6.0-pre1", + "grpc": "^1.6.0", "lodash": "^3.9.3", "google-protobuf": "^3.0.0" }, diff --git a/src/node/tools/package.json b/src/node/tools/package.json index 2a5c1723539..b0b1e8679a5 100644 --- a/src/node/tools/package.json +++ b/src/node/tools/package.json @@ -1,6 +1,6 @@ { "name": "grpc-tools", - "version": "1.6.0-pre1", + "version": "1.6.0", "author": "Google Inc.", "description": "Tools for developing with gRPC on Node.js", "homepage": "https://grpc.io/", diff --git a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec index 7fc065c60c1..0e180a47113 100644 --- a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec +++ b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec @@ -42,7 +42,7 @@ Pod::Spec.new do |s| # exclamation mark ensures that other "regular" pods will be able to find it as it'll be installed # before them. s.name = '!ProtoCompiler-gRPCPlugin' - v = '1.6.0-pre1' + v = '1.6.0' s.version = v s.summary = 'The gRPC ProtoC plugin generates Objective-C files from .proto services.' s.description = <<-DESC diff --git a/src/objective-c/GRPCClient/private/version.h b/src/objective-c/GRPCClient/private/version.h index bd029903913..35215375787 100644 --- a/src/objective-c/GRPCClient/private/version.h +++ b/src/objective-c/GRPCClient/private/version.h @@ -23,4 +23,4 @@ // `tools/buildgen/generate_projects.sh`. -#define GRPC_OBJC_VERSION_STRING @"1.6.0-pre1" +#define GRPC_OBJC_VERSION_STRING @"1.6.0" diff --git a/src/php/ext/grpc/version.h b/src/php/ext/grpc/version.h index 7a20adf8c31..1a46570117f 100644 --- a/src/php/ext/grpc/version.h +++ b/src/php/ext/grpc/version.h @@ -20,6 +20,6 @@ #ifndef VERSION_H #define VERSION_H -#define PHP_GRPC_VERSION "1.6.0RC1" +#define PHP_GRPC_VERSION "1.6.0" #endif /* VERSION_H */ diff --git a/src/python/grpcio/grpc/_grpcio_metadata.py b/src/python/grpcio/grpc/_grpcio_metadata.py index 4be88e1a4fc..28f5a1a8a26 100644 --- a/src/python/grpcio/grpc/_grpcio_metadata.py +++ b/src/python/grpcio/grpc/_grpcio_metadata.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio/grpc/_grpcio_metadata.py.template`!!! -__version__ = """1.6.0rc1""" +__version__ = """1.6.0""" diff --git a/src/python/grpcio/grpc_version.py b/src/python/grpcio/grpc_version.py index 76bf737cfe0..ce35bda91c4 100644 --- a/src/python/grpcio/grpc_version.py +++ b/src/python/grpcio/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio/grpc_version.py.template`!!! -VERSION='1.6.0rc1' +VERSION='1.6.0' diff --git a/src/python/grpcio_health_checking/grpc_version.py b/src/python/grpcio_health_checking/grpc_version.py index 75b0e47da1f..fb5c5f5f651 100644 --- a/src/python/grpcio_health_checking/grpc_version.py +++ b/src/python/grpcio_health_checking/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_health_checking/grpc_version.py.template`!!! -VERSION='1.6.0rc1' +VERSION='1.6.0' diff --git a/src/python/grpcio_reflection/grpc_version.py b/src/python/grpcio_reflection/grpc_version.py index 4194086f405..73e7d2e4977 100644 --- a/src/python/grpcio_reflection/grpc_version.py +++ b/src/python/grpcio_reflection/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_reflection/grpc_version.py.template`!!! -VERSION='1.6.0rc1' +VERSION='1.6.0' diff --git a/src/python/grpcio_tests/grpc_version.py b/src/python/grpcio_tests/grpc_version.py index ca09db0bf0f..99510bbe092 100644 --- a/src/python/grpcio_tests/grpc_version.py +++ b/src/python/grpcio_tests/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_tests/grpc_version.py.template`!!! -VERSION='1.6.0rc1' +VERSION='1.6.0' diff --git a/src/ruby/lib/grpc/version.rb b/src/ruby/lib/grpc/version.rb index fedb824c541..66834d20e16 100644 --- a/src/ruby/lib/grpc/version.rb +++ b/src/ruby/lib/grpc/version.rb @@ -14,5 +14,5 @@ # GRPC contains the General RPC module. module GRPC - VERSION = '1.6.0.pre1' + VERSION = '1.6.0' end diff --git a/src/ruby/tools/version.rb b/src/ruby/tools/version.rb index 4de06c9db6f..73fa15f85f8 100644 --- a/src/ruby/tools/version.rb +++ b/src/ruby/tools/version.rb @@ -14,6 +14,6 @@ module GRPC module Tools - VERSION = '1.6.0.pre1' + VERSION = '1.6.0' end end diff --git a/tools/distrib/python/grpcio_tools/grpc_version.py b/tools/distrib/python/grpcio_tools/grpc_version.py index f1ad8537002..db3b39ae683 100644 --- a/tools/distrib/python/grpcio_tools/grpc_version.py +++ b/tools/distrib/python/grpcio_tools/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/tools/distrib/python/grpcio_tools/grpc_version.py.template`!!! -VERSION='1.6.0rc1' +VERSION='1.6.0' diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index 478a8da685b..f9225688ad6 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -40,7 +40,7 @@ PROJECT_NAME = "GRPC C++" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 1.6.0-pre1 +PROJECT_NUMBER = 1.6.0 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 3756dc92acf..1b41266c715 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -40,7 +40,7 @@ PROJECT_NAME = "GRPC C++" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 1.6.0-pre1 +PROJECT_NUMBER = 1.6.0 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a From 1ebe1e89155159a933ffadc58a07d8aa5e49b844 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Wed, 30 Aug 2017 14:52:57 -0700 Subject: [PATCH 10/41] PHP: fix c++ error after c core 1.6 uptake --- config.m4 | 2 +- templates/config.m4.template | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config.m4 b/config.m4 index f6f8531b2fd..393250fff11 100644 --- a/config.m4 +++ b/config.m4 @@ -12,7 +12,7 @@ if test "$PHP_GRPC" != "no"; then LIBS="-lpthread $LIBS" CFLAGS="-Wall -Werror -Wno-parentheses-equality -Wno-unused-value -std=c11" - CXXFLAGS="-std=c++11" + CXXFLAGS="-std=c++11 -fno-exceptions -fno-rtti" GRPC_SHARED_LIBADD="-lpthread $GRPC_SHARED_LIBADD" PHP_REQUIRE_CXX() PHP_ADD_LIBRARY(pthread) diff --git a/templates/config.m4.template b/templates/config.m4.template index f91893c2bd3..cd93fbd0fbe 100644 --- a/templates/config.m4.template +++ b/templates/config.m4.template @@ -14,7 +14,7 @@ LIBS="-lpthread $LIBS" CFLAGS="-Wall -Werror -Wno-parentheses-equality -Wno-unused-value -std=c11" - CXXFLAGS="-std=c++11" + CXXFLAGS="-std=c++11 -fno-exceptions -fno-rtti" GRPC_SHARED_LIBADD="-lpthread $GRPC_SHARED_LIBADD" PHP_REQUIRE_CXX() PHP_ADD_LIBRARY(pthread) From 9c437c2067e9e0142ad5799696388c2acb25f3d5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 31 Aug 2017 11:23:03 +0200 Subject: [PATCH 11/41] Use docfx to generate C# API reference --- src/csharp/doc/.gitignore | 2 + src/csharp/doc/README.md | 9 ++- src/csharp/doc/docfx.json | 37 ++++++++++ src/csharp/doc/grpc_csharp_public.shfbproj | 83 ---------------------- src/csharp/doc/toc.yml | 3 + 5 files changed, 50 insertions(+), 84 deletions(-) create mode 100644 src/csharp/doc/.gitignore create mode 100644 src/csharp/doc/docfx.json delete mode 100644 src/csharp/doc/grpc_csharp_public.shfbproj create mode 100644 src/csharp/doc/toc.yml diff --git a/src/csharp/doc/.gitignore b/src/csharp/doc/.gitignore new file mode 100644 index 00000000000..09ee235efcd --- /dev/null +++ b/src/csharp/doc/.gitignore @@ -0,0 +1,2 @@ +html +obj diff --git a/src/csharp/doc/README.md b/src/csharp/doc/README.md index 585500b5cab..46cce013a18 100644 --- a/src/csharp/doc/README.md +++ b/src/csharp/doc/README.md @@ -1,2 +1,9 @@ +DocFX-generated C# API Reference +-------------------------------- -SandCastle project files to generate HTML reference documentation. \ No newline at end of file +Install docfx based on instructions here: https://github.com/dotnet/docfx + +``` +# generate docfx documentation into ./html directory +$ docfx +``` diff --git a/src/csharp/doc/docfx.json b/src/csharp/doc/docfx.json new file mode 100644 index 00000000000..7219d0e7a69 --- /dev/null +++ b/src/csharp/doc/docfx.json @@ -0,0 +1,37 @@ +{ + "metadata": [ + { + "src": [ + { + "files": ["Grpc.Core/Grpc.Core.csproj", + "Grpc.Auth/Grpc.Auth.csproj", + "Grpc.Core.Testing/Grpc.Core.Testing.csproj", + "Grpc.HealthCheck/Grpc.HealthCheck.csproj", + "Grpc.Reflection/Grpc.HealthCheck.csproj"], + "exclude": [ "**/bin/**", "**/obj/**" ], + "cwd": ".." + } + ], + "properties": { "TargetFramework": "net45" }, + "dest": "obj/api" + } + ], + "build": { + "content": [ + { + "files": [ "**/*.yml" ], + "cwd": "obj/api", + "dest": "api" + }, + { + "files": [ "toc.yml"], + } + ], + "globalMetadata": { + "_appTitle": "gRPC C#", + "_enableSearch": true, + "_disableContribution": true + }, + "dest": "html" + } +} diff --git a/src/csharp/doc/grpc_csharp_public.shfbproj b/src/csharp/doc/grpc_csharp_public.shfbproj deleted file mode 100644 index fab953da353..00000000000 --- a/src/csharp/doc/grpc_csharp_public.shfbproj +++ /dev/null @@ -1,83 +0,0 @@ - - - - - Debug - AnyCPU - 2.0 - {77e3da09-fc92-486f-a90a-99ca788e8b59} - 2015.6.5.0 - - Documentation - Documentation - Documentation - - .NET Framework 4.5 - ..\..\..\doc\ref\csharp\html - en-US - - - - - - - OnlyWarningsAndErrors - Website - False - True - False - True - 1.0.0.0 - 2 - False - Standard - Blank - True - VS2013 - False - MemberName - gRPC C# - AboveNamespaces - Documentation - - Provides OAuth2 based authentication for gRPC. <c>Grpc.Auth</c> currently consists of a set of very lightweight wrappers and uses C# <a href="https://www.nuget.org/packages/Google.Apis.Auth/">Google.Apis.Auth</a> library. - Main namespace for gRPC C# functionality. Contains concepts representing both client side and server side gRPC logic. - -<seealso cref="Grpc.Core.Channel"/> -<seealso cref="Grpc.Core.Server"/> - Provides functionality to redirect gRPC logs to application-specified destination. - Various utilities for gRPC C#. - - Summary, Parameter, AutoDocumentCtors, Namespace, TypeParameter, AutoDocumentDispose - - - - - - - - - - - - - - - - - - - - - - - - - - - OnBuildSuccess - - \ No newline at end of file diff --git a/src/csharp/doc/toc.yml b/src/csharp/doc/toc.yml new file mode 100644 index 00000000000..c3a1e415ab1 --- /dev/null +++ b/src/csharp/doc/toc.yml @@ -0,0 +1,3 @@ +- name: API Documentation + href: obj/api/ + homepage: obj/api/Grpc.Core.yml From f7acbc40e7e806a8ca3e4793e6ed6831435ab10d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 30 Aug 2017 10:06:28 +0200 Subject: [PATCH 12/41] fix C# distribtest dockerfiles --- .../distribtest/csharp_jessie_x64/Dockerfile | 13 +++++-------- .../distribtest/csharp_jessie_x86/Dockerfile | 13 +++++-------- .../distribtest/csharp_ubuntu1604_x64/Dockerfile | 11 ++++++++--- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/tools/dockerfile/distribtest/csharp_jessie_x64/Dockerfile b/tools/dockerfile/distribtest/csharp_jessie_x64/Dockerfile index d13eecaa551..02ec4c278a7 100644 --- a/tools/dockerfile/distribtest/csharp_jessie_x64/Dockerfile +++ b/tools/dockerfile/distribtest/csharp_jessie_x64/Dockerfile @@ -14,18 +14,15 @@ FROM debian:jessie -RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF -RUN echo "deb http://download.mono-project.com/repo/debian wheezy main" | tee /etc/apt/sources.list.d/mono-xamarin.list +RUN apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF +RUN echo "deb http://download.mono-project.com/repo/debian jessie main" | tee /etc/apt/sources.list.d/mono-official.list RUN echo "deb http://download.mono-project.com/repo/debian wheezy-apache24-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libjpeg62-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list -RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libtiff-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list RUN apt-get update && apt-get install -y \ mono-devel \ ca-certificates-mono \ - nuget + nuget \ + && apt-get clean -# make sure we have nuget 2.12+ (in case there's an older cached docker image) -RUN apt-get update && apt-get install -y nuget - -RUN apt-get update && apt-get install -y unzip +RUN apt-get update && apt-get install -y unzip && apt-get clean diff --git a/tools/dockerfile/distribtest/csharp_jessie_x86/Dockerfile b/tools/dockerfile/distribtest/csharp_jessie_x86/Dockerfile index 71845b590be..758f3145727 100644 --- a/tools/dockerfile/distribtest/csharp_jessie_x86/Dockerfile +++ b/tools/dockerfile/distribtest/csharp_jessie_x86/Dockerfile @@ -14,18 +14,15 @@ FROM 32bit/debian:jessie -RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF -RUN echo "deb http://download.mono-project.com/repo/debian wheezy main" | tee /etc/apt/sources.list.d/mono-xamarin.list +RUN apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF +RUN echo "deb http://download.mono-project.com/repo/debian jessie main" | tee /etc/apt/sources.list.d/mono-official.list RUN echo "deb http://download.mono-project.com/repo/debian wheezy-apache24-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libjpeg62-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list -RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libtiff-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list RUN apt-get update && apt-get install -y \ mono-devel \ ca-certificates-mono \ - nuget + nuget \ + && apt-get clean -# make sure we have nuget 2.12+ (in case there's an older cached docker image) -RUN apt-get update && apt-get install -y nuget - -RUN apt-get update && apt-get install -y unzip +RUN apt-get update && apt-get install -y unzip && apt-get clean diff --git a/tools/dockerfile/distribtest/csharp_ubuntu1604_x64/Dockerfile b/tools/dockerfile/distribtest/csharp_ubuntu1604_x64/Dockerfile index 6604caa42c2..0f40f18e385 100644 --- a/tools/dockerfile/distribtest/csharp_ubuntu1604_x64/Dockerfile +++ b/tools/dockerfile/distribtest/csharp_ubuntu1604_x64/Dockerfile @@ -17,11 +17,16 @@ FROM ubuntu:16.04 RUN apt-get update && apt-get install -y \ mono-devel \ ca-certificates-mono \ - nuget + nuget \ + && apt-get clean # make sure we have nuget 2.12+ (in case there's an older cached docker image) RUN apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF RUN echo "deb http://download.mono-project.com/repo/debian wheezy main" | tee /etc/apt/sources.list.d/mono-xamarin.list -RUN apt-get update && apt-get install -y nuget +RUN apt-get update && apt-get install -y nuget && apt-get clean -RUN apt-get update && apt-get install -y unzip +# Prevent "Error: SendFailure (Error writing headers)" when fetching nuget packages +# See https://github.com/tianon/docker-brew-ubuntu-core/issues/86 +RUN apt-get update && apt-get install -y tzdata && apt-get clean + +RUN apt-get update && apt-get install -y unzip && apt-get clean From 980744c436793ce63cd539c862959fe653df9c35 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 30 Aug 2017 16:36:12 +0200 Subject: [PATCH 13/41] fix C# windows x64 distrib test --- tools/run_tests/artifacts/distribtest_targets.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/artifacts/distribtest_targets.py b/tools/run_tests/artifacts/distribtest_targets.py index fa461efa85e..76407810c05 100644 --- a/tools/run_tests/artifacts/distribtest_targets.py +++ b/tools/run_tests/artifacts/distribtest_targets.py @@ -105,7 +105,9 @@ class CSharpDistribTest(object): use_workspace=True) elif self.platform == 'windows': if self.arch == 'x64': - environ={'MSBUILD_EXTRA_ARGS': '/p:Platform=x64', + # Use double leading / as the first occurence gets removed by msys bash + # when invoking the .bat file (side-effect of posix path conversion) + environ={'MSBUILD_EXTRA_ARGS': '//p:Platform=x64', 'DISTRIBTEST_OUTPATH': 'DistribTest\\bin\\x64\\Debug'} else: environ={'DISTRIBTEST_OUTPATH': 'DistribTest\\bin\\Debug'} From 60c03d9b2a1ecb5d2c494b08e09a606830e2088e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 30 Aug 2017 16:42:16 +0200 Subject: [PATCH 14/41] fix nonexistent version warning for System.Threading.ThreadPool --- src/csharp/Grpc.Core/Grpc.Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/csharp/Grpc.Core/Grpc.Core.csproj b/src/csharp/Grpc.Core/Grpc.Core.csproj index dde800aaddb..08f0e096f10 100755 --- a/src/csharp/Grpc.Core/Grpc.Core.csproj +++ b/src/csharp/Grpc.Core/Grpc.Core.csproj @@ -65,7 +65,7 @@ - + From 9eb36afb1124c9bf1753e78f0efa47bf41d72d71 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 13:40:06 +0200 Subject: [PATCH 15/41] dont set NetStandardImplicitPackageVersion --- src/csharp/Grpc.Auth/Grpc.Auth.csproj | 1 - src/csharp/Grpc.Core.Testing/Grpc.Core.Testing.csproj | 1 - src/csharp/Grpc.Core/Grpc.Core.csproj | 1 - src/csharp/Grpc.HealthCheck/Grpc.HealthCheck.csproj | 1 - src/csharp/Grpc.Reflection/Grpc.Reflection.csproj | 1 - 5 files changed, 5 deletions(-) diff --git a/src/csharp/Grpc.Auth/Grpc.Auth.csproj b/src/csharp/Grpc.Auth/Grpc.Auth.csproj index abf326459c1..bbcbd95be5f 100755 --- a/src/csharp/Grpc.Auth/Grpc.Auth.csproj +++ b/src/csharp/Grpc.Auth/Grpc.Auth.csproj @@ -15,7 +15,6 @@ gRPC RPC Protocol HTTP/2 Auth OAuth2 https://github.com/grpc/grpc https://github.com/grpc/grpc/blob/master/LICENSE - 1.6.0 true true true diff --git a/src/csharp/Grpc.Core.Testing/Grpc.Core.Testing.csproj b/src/csharp/Grpc.Core.Testing/Grpc.Core.Testing.csproj index 9ad6fd0c616..4d6767fa985 100755 --- a/src/csharp/Grpc.Core.Testing/Grpc.Core.Testing.csproj +++ b/src/csharp/Grpc.Core.Testing/Grpc.Core.Testing.csproj @@ -15,7 +15,6 @@ gRPC test testing https://github.com/grpc/grpc https://github.com/grpc/grpc/blob/master/LICENSE - 1.6.0 true true true diff --git a/src/csharp/Grpc.Core/Grpc.Core.csproj b/src/csharp/Grpc.Core/Grpc.Core.csproj index 08f0e096f10..d9950b2f201 100755 --- a/src/csharp/Grpc.Core/Grpc.Core.csproj +++ b/src/csharp/Grpc.Core/Grpc.Core.csproj @@ -14,7 +14,6 @@ gRPC RPC Protocol HTTP/2 https://github.com/grpc/grpc https://github.com/grpc/grpc/blob/master/LICENSE - 1.6.0 true true true diff --git a/src/csharp/Grpc.HealthCheck/Grpc.HealthCheck.csproj b/src/csharp/Grpc.HealthCheck/Grpc.HealthCheck.csproj index 3eb90434f3f..681719d124a 100755 --- a/src/csharp/Grpc.HealthCheck/Grpc.HealthCheck.csproj +++ b/src/csharp/Grpc.HealthCheck/Grpc.HealthCheck.csproj @@ -14,7 +14,6 @@ gRPC health check https://github.com/grpc/grpc https://github.com/grpc/grpc/blob/master/LICENSE - 1.6.0 true true true diff --git a/src/csharp/Grpc.Reflection/Grpc.Reflection.csproj b/src/csharp/Grpc.Reflection/Grpc.Reflection.csproj index b77fd69aeeb..704eea5c17a 100755 --- a/src/csharp/Grpc.Reflection/Grpc.Reflection.csproj +++ b/src/csharp/Grpc.Reflection/Grpc.Reflection.csproj @@ -14,7 +14,6 @@ gRPC reflection https://github.com/grpc/grpc https://github.com/grpc/grpc/blob/master/LICENSE - 1.6.0 true true true From ec6f7280849aa95f30de32fb85d1e27f7c5688fd Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 13:46:48 +0200 Subject: [PATCH 16/41] NUnit.ConsoleRunner dependency is useless --- src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 6df68fda589..be3e312b2a0 100755 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -21,7 +21,6 @@ - From a37b72e9fdee4130af45073714938a39bc83b8e1 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 13:55:46 +0200 Subject: [PATCH 17/41] dont set RuntimeFrameworkVersion unneccessarily --- src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 - .../Grpc.Examples.MathClient/Grpc.Examples.MathClient.csproj | 1 - .../Grpc.Examples.MathServer/Grpc.Examples.MathServer.csproj | 1 - src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj | 1 - src/csharp/Grpc.Examples/Grpc.Examples.csproj | 1 - src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj | 1 - .../Grpc.IntegrationTesting.Client.csproj | 1 - .../Grpc.IntegrationTesting.QpsWorker.csproj | 1 - .../Grpc.IntegrationTesting.Server.csproj | 1 - .../Grpc.IntegrationTesting.StressClient.csproj | 1 - .../Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj | 1 - src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj | 1 - src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj | 1 - 13 files changed, 13 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index be3e312b2a0..82fb080e6ef 100755 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -9,7 +9,6 @@ Exe Grpc.Core.Tests $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.Examples.MathClient/Grpc.Examples.MathClient.csproj b/src/csharp/Grpc.Examples.MathClient/Grpc.Examples.MathClient.csproj index 74deed65840..db4e3ef4e3f 100755 --- a/src/csharp/Grpc.Examples.MathClient/Grpc.Examples.MathClient.csproj +++ b/src/csharp/Grpc.Examples.MathClient/Grpc.Examples.MathClient.csproj @@ -8,7 +8,6 @@ Grpc.Examples.MathClient Exe Grpc.Examples.MathClient - 1.0.4 true diff --git a/src/csharp/Grpc.Examples.MathServer/Grpc.Examples.MathServer.csproj b/src/csharp/Grpc.Examples.MathServer/Grpc.Examples.MathServer.csproj index 1abf2614985..b12b418d015 100755 --- a/src/csharp/Grpc.Examples.MathServer/Grpc.Examples.MathServer.csproj +++ b/src/csharp/Grpc.Examples.MathServer/Grpc.Examples.MathServer.csproj @@ -8,7 +8,6 @@ Grpc.Examples.MathServer Exe Grpc.Examples.MathServer - 1.0.4 true diff --git a/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj b/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj index d2a13ed6e14..32362aad87b 100755 --- a/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj +++ b/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj @@ -9,7 +9,6 @@ Exe Grpc.Examples.Tests $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.Examples/Grpc.Examples.csproj b/src/csharp/Grpc.Examples/Grpc.Examples.csproj index 491d313f178..baa3b4ce6c1 100755 --- a/src/csharp/Grpc.Examples/Grpc.Examples.csproj +++ b/src/csharp/Grpc.Examples/Grpc.Examples.csproj @@ -7,7 +7,6 @@ net45;netcoreapp1.0 Grpc.Examples Grpc.Examples - 1.0.4 true diff --git a/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj b/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj index 2ccf46b9b9c..a9920ff68ec 100755 --- a/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj +++ b/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj @@ -9,7 +9,6 @@ Exe Grpc.HealthCheck.Tests $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj b/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj index c67beea7cd3..0f3971e8d65 100755 --- a/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj +++ b/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj @@ -9,7 +9,6 @@ Exe Grpc.IntegrationTesting.Client $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj b/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj index e452257b1b1..92ed20b0395 100755 --- a/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj +++ b/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj @@ -10,7 +10,6 @@ Grpc.IntegrationTesting.QpsWorker true $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj b/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj index a1fb316fdb1..4480ca1bbaa 100755 --- a/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj +++ b/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj @@ -9,7 +9,6 @@ Exe Grpc.IntegrationTesting.Server $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj b/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj index f64bea3d2be..fef23633ccb 100755 --- a/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj +++ b/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj @@ -9,7 +9,6 @@ Exe Grpc.IntegrationTesting.StressClient $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj index f5077fe0f74..719f1eebba4 100755 --- a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj +++ b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj @@ -9,7 +9,6 @@ Exe Grpc.IntegrationTesting $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj b/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj index 17797e1e1e8..0da24c6179d 100644 --- a/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj +++ b/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj @@ -9,7 +9,6 @@ Exe Grpc.Microbenchmarks $(PackageTargetFallback);portable-net45 - 1.0.4 true diff --git a/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj b/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj index cf756c68ad3..33a9106caed 100755 --- a/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj +++ b/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj @@ -9,7 +9,6 @@ Exe Grpc.Reflection.Tests $(PackageTargetFallback);portable-net45 - 1.0.4 true From f32d3a79b561fc887b91e2bdd5051cf06df502e2 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 13:59:23 +0200 Subject: [PATCH 18/41] System.Linq.Expressions dependency in redundant --- .../Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj index 719f1eebba4..b0de1dc560f 100755 --- a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj +++ b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj @@ -30,10 +30,6 @@ - - - - From 398435122a74f3f6a9f282531bfa91739ac39808 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 14:22:48 +0200 Subject: [PATCH 19/41] PackageTargetFallback not necessary --- src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 - src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj | 1 - src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj | 1 - .../Grpc.IntegrationTesting.Client.csproj | 1 - .../Grpc.IntegrationTesting.QpsWorker.csproj | 1 - .../Grpc.IntegrationTesting.Server.csproj | 1 - .../Grpc.IntegrationTesting.StressClient.csproj | 1 - .../Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj | 1 - src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj | 1 - src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj | 1 - 10 files changed, 10 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 82fb080e6ef..18993a93e00 100755 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -8,7 +8,6 @@ Grpc.Core.Tests Exe Grpc.Core.Tests - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj b/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj index 32362aad87b..3ccc9adfaf7 100755 --- a/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj +++ b/src/csharp/Grpc.Examples.Tests/Grpc.Examples.Tests.csproj @@ -8,7 +8,6 @@ Grpc.Examples.Tests Exe Grpc.Examples.Tests - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj b/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj index a9920ff68ec..9da0539dcb9 100755 --- a/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj +++ b/src/csharp/Grpc.HealthCheck.Tests/Grpc.HealthCheck.Tests.csproj @@ -8,7 +8,6 @@ Grpc.HealthCheck.Tests Exe Grpc.HealthCheck.Tests - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj b/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj index 0f3971e8d65..35713156ea2 100755 --- a/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj +++ b/src/csharp/Grpc.IntegrationTesting.Client/Grpc.IntegrationTesting.Client.csproj @@ -8,7 +8,6 @@ Grpc.IntegrationTesting.Client Exe Grpc.IntegrationTesting.Client - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj b/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj index 92ed20b0395..3ecefe3bc4f 100755 --- a/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj +++ b/src/csharp/Grpc.IntegrationTesting.QpsWorker/Grpc.IntegrationTesting.QpsWorker.csproj @@ -9,7 +9,6 @@ Exe Grpc.IntegrationTesting.QpsWorker true - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj b/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj index 4480ca1bbaa..1092b2c21ef 100755 --- a/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj +++ b/src/csharp/Grpc.IntegrationTesting.Server/Grpc.IntegrationTesting.Server.csproj @@ -8,7 +8,6 @@ Grpc.IntegrationTesting.Server Exe Grpc.IntegrationTesting.Server - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj b/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj index fef23633ccb..22272547f61 100755 --- a/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj +++ b/src/csharp/Grpc.IntegrationTesting.StressClient/Grpc.IntegrationTesting.StressClient.csproj @@ -8,7 +8,6 @@ Grpc.IntegrationTesting.StressClient Exe Grpc.IntegrationTesting.StressClient - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj index b0de1dc560f..c02c9844e3d 100755 --- a/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj +++ b/src/csharp/Grpc.IntegrationTesting/Grpc.IntegrationTesting.csproj @@ -8,7 +8,6 @@ Grpc.IntegrationTesting Exe Grpc.IntegrationTesting - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj b/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj index 0da24c6179d..108357e4eb4 100644 --- a/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj +++ b/src/csharp/Grpc.Microbenchmarks/Grpc.Microbenchmarks.csproj @@ -8,7 +8,6 @@ Grpc.Microbenchmarks Exe Grpc.Microbenchmarks - $(PackageTargetFallback);portable-net45 true diff --git a/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj b/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj index 33a9106caed..d3686971245 100755 --- a/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj +++ b/src/csharp/Grpc.Reflection.Tests/Grpc.Reflection.Tests.csproj @@ -8,7 +8,6 @@ Grpc.Reflection.Tests Exe Grpc.Reflection.Tests - $(PackageTargetFallback);portable-net45 true From c3e2510bd492f6bd30a7f5fa538c0a549058b59d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 5 Sep 2017 15:02:38 +0200 Subject: [PATCH 20/41] csharp_coreclr_x64 dockerfile not useful --- .../test/csharp_coreclr_x64/Dockerfile | 114 ------------------ tools/run_tests/artifacts/package_targets.py | 2 +- 2 files changed, 1 insertion(+), 115 deletions(-) delete mode 100644 tools/dockerfile/test/csharp_coreclr_x64/Dockerfile diff --git a/tools/dockerfile/test/csharp_coreclr_x64/Dockerfile b/tools/dockerfile/test/csharp_coreclr_x64/Dockerfile deleted file mode 100644 index 4ccfbc43c30..00000000000 --- a/tools/dockerfile/test/csharp_coreclr_x64/Dockerfile +++ /dev/null @@ -1,114 +0,0 @@ -# Copyright 2015 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. - -FROM debian:jessie - -# Install Git and basic packages. -RUN apt-get update && apt-get install -y \ - autoconf \ - autotools-dev \ - build-essential \ - bzip2 \ - ccache \ - curl \ - gcc \ - gcc-multilib \ - git \ - golang \ - gyp \ - lcov \ - libc6 \ - libc6-dbg \ - libc6-dev \ - libgtest-dev \ - libtool \ - make \ - perl \ - strace \ - python-dev \ - python-setuptools \ - python-yaml \ - telnet \ - unzip \ - wget \ - zip && apt-get clean - -#================ -# Build profiling -RUN apt-get update && apt-get install -y time && apt-get clean - -#==================== -# Python dependencies - -# Install dependencies - -RUN apt-get update && apt-get install -y \ - python-all-dev \ - python3-all-dev \ - python-pip - -# Install Python packages from PyPI -RUN pip install pip --upgrade -RUN pip install virtualenv -RUN pip install futures==2.2.0 enum34==1.0.4 protobuf==3.2.0 six==1.10.0 - -#================ -# C# dependencies - -# Update to a newer version of mono -RUN apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF -RUN echo "deb http://download.mono-project.com/repo/debian jessie main" | tee /etc/apt/sources.list.d/mono-official.list -RUN echo "deb http://download.mono-project.com/repo/debian wheezy-apache24-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list -RUN echo "deb http://download.mono-project.com/repo/debian wheezy-libjpeg62-compat main" | tee -a /etc/apt/sources.list.d/mono-xamarin.list - -# Install dependencies -RUN apt-get update && apt-get -y dist-upgrade && apt-get install -y \ - mono-devel \ - ca-certificates-mono \ - nuget \ - && apt-get clean - -RUN nuget update -self - -# Install dotnet SDK based on https://www.microsoft.com/net/core#debian -RUN apt-get update && apt-get install -y curl libunwind8 gettext -# dotnet-dev-1.0.0-preview2-003131 -RUN curl -sSL -o dotnet100.tar.gz https://go.microsoft.com/fwlink/?LinkID=827530 -RUN mkdir -p /opt/dotnet && tar zxf dotnet100.tar.gz -C /opt/dotnet -# dotnet-dev-1.0.1 -RUN curl -sSL -o dotnet101.tar.gz https://go.microsoft.com/fwlink/?LinkID=843453 -RUN mkdir -p /opt/dotnet && tar zxf dotnet101.tar.gz -C /opt/dotnet -RUN ln -s /opt/dotnet/dotnet /usr/local/bin - -# Trigger the population of the local package cache -ENV NUGET_XMLDOC_MODE skip -RUN mkdir warmup \ - && cd warmup \ - && dotnet new \ - && cd .. \ - && rm -rf warmup - -# Prepare ccache -RUN ln -s /usr/bin/ccache /usr/local/bin/gcc -RUN ln -s /usr/bin/ccache /usr/local/bin/g++ -RUN ln -s /usr/bin/ccache /usr/local/bin/cc -RUN ln -s /usr/bin/ccache /usr/local/bin/c++ -RUN ln -s /usr/bin/ccache /usr/local/bin/clang -RUN ln -s /usr/bin/ccache /usr/local/bin/clang++ - - -RUN mkdir /var/local/jenkins - -# Define the default command. -CMD ["bash"] diff --git a/tools/run_tests/artifacts/package_targets.py b/tools/run_tests/artifacts/package_targets.py index 0da13864f0c..671d0f7b45e 100644 --- a/tools/run_tests/artifacts/package_targets.py +++ b/tools/run_tests/artifacts/package_targets.py @@ -78,7 +78,7 @@ class CSharpPackage: if self.linux: return create_docker_jobspec( self.name, - 'tools/dockerfile/test/csharp_coreclr_x64', + 'tools/dockerfile/test/csharp_jessie_x64', 'src/csharp/build_packages_dotnetcli.sh') else: return create_jobspec(self.name, From eda5a4db1e88e484d0c45401c345cbc199d633fd Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Sat, 9 Sep 2017 15:52:11 +0000 Subject: [PATCH 21/41] Fix metadata memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gRPC Core has two styles for passing metadata: as an integer count along with a grpc_metadata* pointer, which is used for passing metadata into the core, and as a grpc_metadata_array, which is used for passing metadata out of the core. The Cython layer of gRPC Python was using a single data structure wrapping grpc_metadata_array for both purposes, but this was complex because the core manages the slices contained in grpc_metadata_array objects (at least those of which it is aware), so the Cython layer had to keep track of whether or not the core was aware of the slices it was using (and it was also defective, leaking slices). This is solved by realigning with the Cython layer’s intended design of mirroring as closely as possible in Python the gRPC Core API: we use one structure for passing metadata into the core (what is now called cygrpc.Metadata) and second, different structure for receiving metadata out of the core (what was called cygrpc.Metadata but is now cygrpc.MetadataArray, reflecting that it wraps the core’s grpc_metadata_array). All bug fixes should contain added tests preventing regression but this doesn't because I don't know at this time how to write a does-not-leak test for Python that fits well into our existing body of tests. Phooey. Thanks to Dominik Janků (djanku@email.cz) for investigation and an earlier draft of a solution. --- .../_cython/_cygrpc/completion_queue.pyx.pxi | 11 +- .../grpc/_cython/_cygrpc/credentials.pyx.pxi | 9 +- .../grpcio/grpc/_cython/_cygrpc/grpc.pxi | 1 + .../grpc/_cython/_cygrpc/records.pxd.pxi | 13 +- .../grpc/_cython/_cygrpc/records.pyx.pxi | 120 ++++++++++-------- .../grpc/_cython/_cygrpc/server.pyx.pxi | 2 +- 6 files changed, 84 insertions(+), 72 deletions(-) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi index 28c30e5d357..237f4307990 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/completion_queue.pyx.pxi @@ -41,9 +41,8 @@ cdef class CompletionQueue: cdef object user_tag = None cdef Call operation_call = None cdef CallDetails request_call_details = None - cdef Metadata request_metadata = None + cdef object request_metadata = None cdef Operations batch_operations = None - cdef Operation batch_operation = None if event.type == GRPC_QUEUE_TIMEOUT: return Event( event.type, False, None, None, None, None, False, None) @@ -63,14 +62,8 @@ cdef class CompletionQueue: operation_call = tag.operation_call request_call_details = tag.request_call_details if tag.request_metadata is not None: - request_metadata = tag.request_metadata - request_metadata._claim_slice_ownership() + request_metadata = tuple(tag.request_metadata) batch_operations = tag.batch_operations - if tag.batch_operations is not None: - for op in batch_operations.operations: - batch_operation = op - if batch_operation._received_metadata is not None: - batch_operation._received_metadata._claim_slice_ownership() if tag.is_new_request: # Stuff in the tag not explicitly handled by us needs to live through # the life of the call diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi index 98d7a9820df..57816f1cab0 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/credentials.pyx.pxi @@ -76,7 +76,7 @@ cdef class CredentialsMetadataPlugin: """ Args: plugin_callback (callable): Callback accepting a service URL (str/bytes) - and callback object (accepting a Metadata, + and callback object (accepting a MetadataArray, grpc_status_code, and a str/bytes error message). This argument when called should be non-blocking and eventually call the callback object with the appropriate status code/details and metadata (if @@ -129,8 +129,7 @@ cdef void plugin_get_metadata( def python_callback( Metadata metadata, grpc_status_code status, bytes error_details): - cb(user_data, metadata.c_metadata_array.metadata, - metadata.c_metadata_array.count, status, error_details) + cb(user_data, metadata.c_metadata, metadata.c_count, status, error_details) called_flag[0] = True cdef CredentialsMetadataPlugin self = state cdef AuthMetadataContext cy_context = AuthMetadataContext() @@ -139,8 +138,8 @@ cdef void plugin_get_metadata( self.plugin_callback(cy_context, python_callback) except Exception as error: if not called_flag[0]: - cb(user_data, Metadata([]).c_metadata_array.metadata, - 0, StatusCode.unknown, traceback.format_exc().encode()) + cb(user_data, NULL, 0, StatusCode.unknown, + traceback.format_exc().encode()) cdef void plugin_destroy_c_plugin_state(void *state) with gil: cpython.Py_DECREF(state) diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi index 5950bfa0e6a..28cf1144510 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/grpc.pxi @@ -59,6 +59,7 @@ cdef extern from "grpc/grpc.h": grpc_slice grpc_slice_malloc(size_t length) nogil grpc_slice grpc_slice_from_copied_string(const char *source) nogil grpc_slice grpc_slice_from_copied_buffer(const char *source, size_t len) nogil + grpc_slice grpc_slice_copy(grpc_slice s) nogil # Declare functions for function-like macros (because Cython)... void *grpc_slice_start_ptr "GRPC_SLICE_START_PTR" (grpc_slice s) nogil diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/records.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/records.pxd.pxi index 8ace6aeb525..9c40ebf0c28 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/records.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/records.pxd.pxi @@ -37,7 +37,7 @@ cdef class OperationTag: cdef Server shutting_down_server cdef Call operation_call cdef CallDetails request_call_details - cdef Metadata request_metadata + cdef MetadataArray request_metadata cdef Operations batch_operations cdef bint is_new_request @@ -51,7 +51,7 @@ cdef class Event: # For Server.request_call cdef readonly bint is_new_request cdef readonly CallDetails request_call_details - cdef readonly Metadata request_metadata + cdef readonly object request_metadata # For server calls cdef readonly Call operation_call @@ -92,15 +92,20 @@ cdef class Metadatum: cdef class Metadata: + cdef grpc_metadata *c_metadata + cdef readonly size_t c_count + + +cdef class MetadataArray: + cdef grpc_metadata_array c_metadata_array - cdef void _claim_slice_ownership(self) cdef class Operation: cdef grpc_op c_op cdef ByteBuffer _received_message - cdef Metadata _received_metadata + cdef MetadataArray _received_metadata cdef grpc_status_code _received_status_code cdef grpc_slice _status_details cdef int _received_cancelled diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi index 1b2ddd2469c..0a2a6eee051 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/records.pyx.pxi @@ -238,7 +238,7 @@ cdef class Event: def __cinit__(self, grpc_completion_type type, bint success, object tag, Call operation_call, CallDetails request_call_details, - Metadata request_metadata, + object request_metadata, bint is_new_request, Operations batch_operations): self.type = type @@ -437,48 +437,79 @@ cdef class Metadatum: cdef class _MetadataIterator: cdef size_t i - cdef Metadata metadata + cdef size_t _length + cdef object _metadatum_indexable - def __cinit__(self, Metadata metadata not None): + def __cinit__(self, length, metadatum_indexable): + self._length = length + self._metadatum_indexable = metadatum_indexable self.i = 0 - self.metadata = metadata def __iter__(self): return self def __next__(self): - if self.i < len(self.metadata): - result = self.metadata[self.i] + if self.i < self._length: + result = self._metadatum_indexable[self.i] self.i = self.i + 1 return result else: raise StopIteration +# TODO(https://github.com/grpc/grpc/issues/7950): Eliminate this; just use an +# ordinary sequence of pairs of bytestrings all the way down to the +# grpc_call_start_batch call. cdef class Metadata: + """Metadata being passed from application to core.""" def __cinit__(self, metadata_iterable): + metadata_sequence = tuple(metadata_iterable) + cdef size_t count = len(metadata_sequence) with nogil: grpc_init() - grpc_metadata_array_init(&self.c_metadata_array) - metadata = list(metadata_iterable) - for metadatum in metadata: - if not isinstance(metadatum, Metadatum): - raise TypeError("expected list of Metadatum") - self.c_metadata_array.count = len(metadata) - self.c_metadata_array.capacity = len(metadata) - with nogil: - self.c_metadata_array.metadata = gpr_malloc( - self.c_metadata_array.count*sizeof(grpc_metadata) - ) - for i in range(self.c_metadata_array.count): - (metadata[i])._copy_metadatum(&self.c_metadata_array.metadata[i]) + self.c_metadata = gpr_malloc( + count * sizeof(grpc_metadata)) + self.c_count = count + for index, metadatum in enumerate(metadata_sequence): + self.c_metadata[index].key = grpc_slice_copy( + (metadatum).c_metadata.key) + self.c_metadata[index].value = grpc_slice_copy( + (metadatum).c_metadata.value) + + def __dealloc__(self): + with nogil: + for index in range(self.c_count): + grpc_slice_unref(self.c_metadata[index].key) + grpc_slice_unref(self.c_metadata[index].value) + gpr_free(self.c_metadata) + grpc_shutdown() + + def __len__(self): + return self.c_count + + def __getitem__(self, size_t index): + if index < self.c_count: + key = _slice_bytes(self.c_metadata[index].key) + value = _slice_bytes(self.c_metadata[index].value) + return Metadatum(key, value) + else: + raise IndexError() + + def __iter__(self): + return _MetadataIterator(self.c_count, self) + + +cdef class MetadataArray: + """Metadata being passed from core to application.""" + + def __cinit__(self): + with nogil: + grpc_init() + grpc_metadata_array_init(&self.c_metadata_array) def __dealloc__(self): with nogil: - # this frees the allocated memory for the grpc_metadata_array (although - # it'd be nice if that were documented somewhere...) - # TODO(atash): document this in the C core grpc_metadata_array_destroy(&self.c_metadata_array) grpc_shutdown() @@ -493,21 +524,7 @@ cdef class Metadata: return Metadatum(key=key, value=value) def __iter__(self): - return _MetadataIterator(self) - - cdef void _claim_slice_ownership(self): - cdef grpc_metadata_array new_c_metadata_array - grpc_metadata_array_init(&new_c_metadata_array) - new_c_metadata_array.metadata = gpr_malloc( - self.c_metadata_array.count*sizeof(grpc_metadata)) - new_c_metadata_array.count = self.c_metadata_array.count - for i in range(self.c_metadata_array.count): - new_c_metadata_array.metadata[i].key = _copy_slice( - self.c_metadata_array.metadata[i].key) - new_c_metadata_array.metadata[i].value = _copy_slice( - self.c_metadata_array.metadata[i].value) - grpc_metadata_array_destroy(&self.c_metadata_array) - self.c_metadata_array = new_c_metadata_array + return _MetadataIterator(self.c_metadata_array.count, self) cdef class Operation: @@ -547,14 +564,13 @@ cdef class Operation: if (self.c_op.type != GRPC_OP_RECV_INITIAL_METADATA and self.c_op.type != GRPC_OP_RECV_STATUS_ON_CLIENT): raise TypeError("self must be an operation receiving metadata") - return self._received_metadata - - @property - def received_metadata_or_none(self): - if (self.c_op.type != GRPC_OP_RECV_INITIAL_METADATA and - self.c_op.type != GRPC_OP_RECV_STATUS_ON_CLIENT): - return None - return self._received_metadata + # TODO(https://github.com/grpc/grpc/issues/7950): Drop the "all Cython + # objects must be legitimate for use from Python at any time" policy in + # place today, shift the policy toward "Operation objects are only usable + # while their calls are active", and move this making-a-copy-because-this- + # data-needs-to-live-much-longer-than-the-call-from-which-it-arose to the + # lowest Python layer. + return tuple(self._received_metadata) @property def received_status_code(self): @@ -601,9 +617,8 @@ def operation_send_initial_metadata(Metadata metadata, int flags): cdef Operation op = Operation() op.c_op.type = GRPC_OP_SEND_INITIAL_METADATA op.c_op.flags = flags - op.c_op.data.send_initial_metadata.count = metadata.c_metadata_array.count - op.c_op.data.send_initial_metadata.metadata = ( - metadata.c_metadata_array.metadata) + op.c_op.data.send_initial_metadata.count = metadata.c_count + op.c_op.data.send_initial_metadata.metadata = metadata.c_metadata op.references.append(metadata) op.is_valid = True return op @@ -631,9 +646,8 @@ def operation_send_status_from_server( op.c_op.type = GRPC_OP_SEND_STATUS_FROM_SERVER op.c_op.flags = flags op.c_op.data.send_status_from_server.trailing_metadata_count = ( - metadata.c_metadata_array.count) - op.c_op.data.send_status_from_server.trailing_metadata = ( - metadata.c_metadata_array.metadata) + metadata.c_count) + op.c_op.data.send_status_from_server.trailing_metadata = metadata.c_metadata op.c_op.data.send_status_from_server.status = code grpc_slice_unref(op._status_details) op._status_details = _slice_from_bytes(details) @@ -646,7 +660,7 @@ def operation_receive_initial_metadata(int flags): cdef Operation op = Operation() op.c_op.type = GRPC_OP_RECV_INITIAL_METADATA op.c_op.flags = flags - op._received_metadata = Metadata([]) + op._received_metadata = MetadataArray() op.c_op.data.receive_initial_metadata.receive_initial_metadata = ( &op._received_metadata.c_metadata_array) op.is_valid = True @@ -669,7 +683,7 @@ def operation_receive_status_on_client(int flags): cdef Operation op = Operation() op.c_op.type = GRPC_OP_RECV_STATUS_ON_CLIENT op.c_op.flags = flags - op._received_metadata = Metadata([]) + op._received_metadata = MetadataArray() op.c_op.data.receive_status_on_client.trailing_metadata = ( &op._received_metadata.c_metadata_array) op.c_op.data.receive_status_on_client.status = ( diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi index dd276fd57b5..b8db27469f9 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/server.pyx.pxi @@ -44,7 +44,7 @@ cdef class Server: cdef OperationTag operation_tag = OperationTag(tag) operation_tag.operation_call = Call() operation_tag.request_call_details = CallDetails() - operation_tag.request_metadata = Metadata([]) + operation_tag.request_metadata = MetadataArray() operation_tag.references.extend([self, call_queue, server_queue]) operation_tag.is_new_request = True operation_tag.batch_operations = Operations([]) From 4fa33649753f64283c82f73f5ca59daaa8895671 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Sep 2017 12:56:45 +0200 Subject: [PATCH 22/41] bump version to 1.6.1 --- BUILD | 2 +- CMakeLists.txt | 2 +- Makefile | 4 ++-- build.yaml | 2 +- gRPC-Core.podspec | 2 +- gRPC-ProtoRPC.podspec | 2 +- gRPC-RxLibrary.podspec | 2 +- gRPC.podspec | 2 +- package.json | 2 +- package.xml | 4 ++-- src/cpp/common/version_cc.cc | 2 +- src/csharp/Grpc.Core/Version.csproj.include | 2 +- src/csharp/Grpc.Core/VersionInfo.cs | 4 ++-- src/csharp/build_packages_dotnetcli.bat | 2 +- src/csharp/build_packages_dotnetcli.sh | 4 ++-- src/node/health_check/package.json | 4 ++-- src/node/tools/package.json | 2 +- src/objective-c/!ProtoCompiler-gRPCPlugin.podspec | 2 +- src/objective-c/GRPCClient/private/version.h | 2 +- src/php/composer.json | 2 +- src/php/ext/grpc/version.h | 2 +- src/python/grpcio/grpc/_grpcio_metadata.py | 2 +- src/python/grpcio/grpc_version.py | 2 +- src/python/grpcio_health_checking/grpc_version.py | 2 +- src/python/grpcio_reflection/grpc_version.py | 2 +- src/python/grpcio_tests/grpc_version.py | 2 +- src/ruby/lib/grpc/version.rb | 2 +- src/ruby/tools/version.rb | 2 +- tools/distrib/python/grpcio_tools/grpc_version.py | 2 +- tools/doxygen/Doxyfile.c++ | 2 +- tools/doxygen/Doxyfile.c++.internal | 2 +- 31 files changed, 36 insertions(+), 36 deletions(-) diff --git a/BUILD b/BUILD index 4d1121a79d5..cbd7e46b970 100644 --- a/BUILD +++ b/BUILD @@ -38,7 +38,7 @@ g_stands_for = "garcia" core_version = "4.0.0" -version = "1.6.0" +version = "1.6.1" GPR_PUBLIC_HDRS = [ "include/grpc/support/alloc.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index ebbaa6a9ec7..5225c6e34e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,7 +24,7 @@ cmake_minimum_required(VERSION 2.8) set(PACKAGE_NAME "grpc") -set(PACKAGE_VERSION "1.6.0") +set(PACKAGE_VERSION "1.6.1") set(PACKAGE_STRING "${PACKAGE_NAME} ${PACKAGE_VERSION}") set(PACKAGE_TARNAME "${PACKAGE_NAME}-${PACKAGE_VERSION}") set(PACKAGE_BUGREPORT "https://github.com/grpc/grpc/issues/") diff --git a/Makefile b/Makefile index 801f8a7d6ed..5feff15071e 100644 --- a/Makefile +++ b/Makefile @@ -411,8 +411,8 @@ Q = @ endif CORE_VERSION = 4.0.0 -CPP_VERSION = 1.6.0 -CSHARP_VERSION = 1.6.0 +CPP_VERSION = 1.6.1 +CSHARP_VERSION = 1.6.1 CPPFLAGS_NO_ARCH += $(addprefix -I, $(INCLUDES)) $(addprefix -D, $(DEFINES)) CPPFLAGS += $(CPPFLAGS_NO_ARCH) $(ARCH_FLAGS) diff --git a/build.yaml b/build.yaml index 723c8fe4460..954d6a3ecd7 100644 --- a/build.yaml +++ b/build.yaml @@ -14,7 +14,7 @@ settings: '#10': See the expand_version.py for all the quirks here core_version: 4.0.0 g_stands_for: garcia - version: 1.6.0 + version: 1.6.1 filegroups: - name: census public_headers: diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 222d3e4f0f5..1b90beffc46 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -22,7 +22,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-Core' - version = '1.6.0' + version = '1.6.1' s.version = version s.summary = 'Core cross-platform gRPC library, written in C' s.homepage = 'https://grpc.io' diff --git a/gRPC-ProtoRPC.podspec b/gRPC-ProtoRPC.podspec index 2d2f31d3239..f060274b5f7 100644 --- a/gRPC-ProtoRPC.podspec +++ b/gRPC-ProtoRPC.podspec @@ -21,7 +21,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-ProtoRPC' - version = '1.6.0' + version = '1.6.1' s.version = version s.summary = 'RPC library for Protocol Buffers, based on gRPC' s.homepage = 'https://grpc.io' diff --git a/gRPC-RxLibrary.podspec b/gRPC-RxLibrary.podspec index 38a159404fd..76bf4004248 100644 --- a/gRPC-RxLibrary.podspec +++ b/gRPC-RxLibrary.podspec @@ -21,7 +21,7 @@ Pod::Spec.new do |s| s.name = 'gRPC-RxLibrary' - version = '1.6.0' + version = '1.6.1' s.version = version s.summary = 'Reactive Extensions library for iOS/OSX.' s.homepage = 'https://grpc.io' diff --git a/gRPC.podspec b/gRPC.podspec index 12591cda336..3c310c8cd9d 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -20,7 +20,7 @@ Pod::Spec.new do |s| s.name = 'gRPC' - version = '1.6.0' + version = '1.6.1' s.version = version s.summary = 'gRPC client library for iOS/OSX' s.homepage = 'https://grpc.io' diff --git a/package.json b/package.json index 94e6391a8d8..dd383548f38 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "grpc", - "version": "1.6.0", + "version": "1.6.1", "author": "Google Inc.", "description": "gRPC Library for Node", "homepage": "https://grpc.io/", diff --git a/package.xml b/package.xml index 30018620dd7..0735c02944c 100644 --- a/package.xml +++ b/package.xml @@ -13,8 +13,8 @@ 2017-08-24 - 1.6.0 - 1.6.0 + 1.6.1 + 1.6.1 beta diff --git a/src/cpp/common/version_cc.cc b/src/cpp/common/version_cc.cc index c65185c77ee..35c1ec6f5e7 100644 --- a/src/cpp/common/version_cc.cc +++ b/src/cpp/common/version_cc.cc @@ -22,5 +22,5 @@ #include namespace grpc { -grpc::string Version() { return "1.6.0"; } +grpc::string Version() { return "1.6.1"; } } diff --git a/src/csharp/Grpc.Core/Version.csproj.include b/src/csharp/Grpc.Core/Version.csproj.include index 23f6e409091..616f390f888 100755 --- a/src/csharp/Grpc.Core/Version.csproj.include +++ b/src/csharp/Grpc.Core/Version.csproj.include @@ -1,7 +1,7 @@ - 1.6.0 + 1.6.1 3.3.0 diff --git a/src/csharp/Grpc.Core/VersionInfo.cs b/src/csharp/Grpc.Core/VersionInfo.cs index d402c323c1f..52f8e2642aa 100644 --- a/src/csharp/Grpc.Core/VersionInfo.cs +++ b/src/csharp/Grpc.Core/VersionInfo.cs @@ -33,11 +33,11 @@ namespace Grpc.Core /// /// Current AssemblyFileVersion of gRPC C# assemblies /// - public const string CurrentAssemblyFileVersion = "1.6.0.0"; + public const string CurrentAssemblyFileVersion = "1.6.1.0"; /// /// Current version of gRPC C# /// - public const string CurrentVersion = "1.6.0"; + public const string CurrentVersion = "1.6.1"; } } diff --git a/src/csharp/build_packages_dotnetcli.bat b/src/csharp/build_packages_dotnetcli.bat index b7aa22b2928..a6581ac7336 100755 --- a/src/csharp/build_packages_dotnetcli.bat +++ b/src/csharp/build_packages_dotnetcli.bat @@ -13,7 +13,7 @@ @rem limitations under the License. @rem Current package versions -set VERSION=1.6.0 +set VERSION=1.6.1 @rem Adjust the location of nuget.exe set NUGET=C:\nuget\nuget.exe diff --git a/src/csharp/build_packages_dotnetcli.sh b/src/csharp/build_packages_dotnetcli.sh index 455b930525b..76d6544c07b 100755 --- a/src/csharp/build_packages_dotnetcli.sh +++ b/src/csharp/build_packages_dotnetcli.sh @@ -39,7 +39,7 @@ dotnet pack --configuration Release Grpc.Auth --output ../../../artifacts dotnet pack --configuration Release Grpc.HealthCheck --output ../../../artifacts dotnet pack --configuration Release Grpc.Reflection --output ../../../artifacts -nuget pack Grpc.nuspec -Version "1.6.0" -OutputDirectory ../../artifacts -nuget pack Grpc.Tools.nuspec -Version "1.6.0" -OutputDirectory ../../artifacts +nuget pack Grpc.nuspec -Version "1.6.1" -OutputDirectory ../../artifacts +nuget pack Grpc.Tools.nuspec -Version "1.6.1" -OutputDirectory ../../artifacts (cd ../../artifacts && zip csharp_nugets_dotnetcli.zip *.nupkg) diff --git a/src/node/health_check/package.json b/src/node/health_check/package.json index 66ab2a292fe..92e748e72f0 100644 --- a/src/node/health_check/package.json +++ b/src/node/health_check/package.json @@ -1,6 +1,6 @@ { "name": "grpc-health-check", - "version": "1.6.0", + "version": "1.6.1", "author": "Google Inc.", "description": "Health check service for use with gRPC", "repository": { @@ -15,7 +15,7 @@ } ], "dependencies": { - "grpc": "^1.6.0", + "grpc": "^1.6.1", "lodash": "^3.9.3", "google-protobuf": "^3.0.0" }, diff --git a/src/node/tools/package.json b/src/node/tools/package.json index b0b1e8679a5..023cb1e4640 100644 --- a/src/node/tools/package.json +++ b/src/node/tools/package.json @@ -1,6 +1,6 @@ { "name": "grpc-tools", - "version": "1.6.0", + "version": "1.6.1", "author": "Google Inc.", "description": "Tools for developing with gRPC on Node.js", "homepage": "https://grpc.io/", diff --git a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec index 0e180a47113..32f6c39e6cb 100644 --- a/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec +++ b/src/objective-c/!ProtoCompiler-gRPCPlugin.podspec @@ -42,7 +42,7 @@ Pod::Spec.new do |s| # exclamation mark ensures that other "regular" pods will be able to find it as it'll be installed # before them. s.name = '!ProtoCompiler-gRPCPlugin' - v = '1.6.0' + v = '1.6.1' s.version = v s.summary = 'The gRPC ProtoC plugin generates Objective-C files from .proto services.' s.description = <<-DESC diff --git a/src/objective-c/GRPCClient/private/version.h b/src/objective-c/GRPCClient/private/version.h index 35215375787..64e08f9151b 100644 --- a/src/objective-c/GRPCClient/private/version.h +++ b/src/objective-c/GRPCClient/private/version.h @@ -23,4 +23,4 @@ // `tools/buildgen/generate_projects.sh`. -#define GRPC_OBJC_VERSION_STRING @"1.6.0" +#define GRPC_OBJC_VERSION_STRING @"1.6.1" diff --git a/src/php/composer.json b/src/php/composer.json index 5d492ccb86d..df3dff39228 100644 --- a/src/php/composer.json +++ b/src/php/composer.json @@ -2,7 +2,7 @@ "name": "grpc/grpc-dev", "description": "gRPC library for PHP - for Developement use only", "license": "Apache-2.0", - "version": "1.6.0", + "version": "1.6.1", "require": { "php": ">=5.5.0", "google/protobuf": "^v3.3.0" diff --git a/src/php/ext/grpc/version.h b/src/php/ext/grpc/version.h index 1a46570117f..67f0c4b5ee8 100644 --- a/src/php/ext/grpc/version.h +++ b/src/php/ext/grpc/version.h @@ -20,6 +20,6 @@ #ifndef VERSION_H #define VERSION_H -#define PHP_GRPC_VERSION "1.6.0" +#define PHP_GRPC_VERSION "1.6.1" #endif /* VERSION_H */ diff --git a/src/python/grpcio/grpc/_grpcio_metadata.py b/src/python/grpcio/grpc/_grpcio_metadata.py index 28f5a1a8a26..8707a5a9a62 100644 --- a/src/python/grpcio/grpc/_grpcio_metadata.py +++ b/src/python/grpcio/grpc/_grpcio_metadata.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio/grpc/_grpcio_metadata.py.template`!!! -__version__ = """1.6.0""" +__version__ = """1.6.1""" diff --git a/src/python/grpcio/grpc_version.py b/src/python/grpcio/grpc_version.py index ce35bda91c4..5a76a66e6cc 100644 --- a/src/python/grpcio/grpc_version.py +++ b/src/python/grpcio/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio/grpc_version.py.template`!!! -VERSION='1.6.0' +VERSION='1.6.1' diff --git a/src/python/grpcio_health_checking/grpc_version.py b/src/python/grpcio_health_checking/grpc_version.py index fb5c5f5f651..41f7e344baf 100644 --- a/src/python/grpcio_health_checking/grpc_version.py +++ b/src/python/grpcio_health_checking/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_health_checking/grpc_version.py.template`!!! -VERSION='1.6.0' +VERSION='1.6.1' diff --git a/src/python/grpcio_reflection/grpc_version.py b/src/python/grpcio_reflection/grpc_version.py index 73e7d2e4977..63865b1bb37 100644 --- a/src/python/grpcio_reflection/grpc_version.py +++ b/src/python/grpcio_reflection/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_reflection/grpc_version.py.template`!!! -VERSION='1.6.0' +VERSION='1.6.1' diff --git a/src/python/grpcio_tests/grpc_version.py b/src/python/grpcio_tests/grpc_version.py index 99510bbe092..ea0278c7e52 100644 --- a/src/python/grpcio_tests/grpc_version.py +++ b/src/python/grpcio_tests/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_tests/grpc_version.py.template`!!! -VERSION='1.6.0' +VERSION='1.6.1' diff --git a/src/ruby/lib/grpc/version.rb b/src/ruby/lib/grpc/version.rb index 66834d20e16..680e5d1454d 100644 --- a/src/ruby/lib/grpc/version.rb +++ b/src/ruby/lib/grpc/version.rb @@ -14,5 +14,5 @@ # GRPC contains the General RPC module. module GRPC - VERSION = '1.6.0' + VERSION = '1.6.1' end diff --git a/src/ruby/tools/version.rb b/src/ruby/tools/version.rb index 73fa15f85f8..2d2db885756 100644 --- a/src/ruby/tools/version.rb +++ b/src/ruby/tools/version.rb @@ -14,6 +14,6 @@ module GRPC module Tools - VERSION = '1.6.0' + VERSION = '1.6.1' end end diff --git a/tools/distrib/python/grpcio_tools/grpc_version.py b/tools/distrib/python/grpcio_tools/grpc_version.py index db3b39ae683..0630445fd48 100644 --- a/tools/distrib/python/grpcio_tools/grpc_version.py +++ b/tools/distrib/python/grpcio_tools/grpc_version.py @@ -14,4 +14,4 @@ # AUTO-GENERATED FROM `$REPO_ROOT/templates/tools/distrib/python/grpcio_tools/grpc_version.py.template`!!! -VERSION='1.6.0' +VERSION='1.6.1' diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index f9225688ad6..cefce74f98e 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -40,7 +40,7 @@ PROJECT_NAME = "GRPC C++" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 1.6.0 +PROJECT_NUMBER = 1.6.1 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 1b41266c715..c9bdd1454d6 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -40,7 +40,7 @@ PROJECT_NAME = "GRPC C++" # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 1.6.0 +PROJECT_NUMBER = 1.6.1 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a From 44a2563cab51ea20cf729a98400e2f8287c953fb Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Sep 2017 16:55:23 +0200 Subject: [PATCH 23/41] add missing template for grpcio_testing versining --- .../grpcio_testing/grpc_version.py.template | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 templates/src/python/grpcio_testing/grpc_version.py.template diff --git a/templates/src/python/grpcio_testing/grpc_version.py.template b/templates/src/python/grpcio_testing/grpc_version.py.template new file mode 100644 index 00000000000..74db811d60e --- /dev/null +++ b/templates/src/python/grpcio_testing/grpc_version.py.template @@ -0,0 +1,19 @@ +%YAML 1.2 +--- | + # 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. + + # AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_testing/grpc_version.py.template`!!! + + VERSION='${settings.python_version.pep440()}' From ea7807876cb7a4c53e610f516d6dbc09d1ef2d2e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Sep 2017 16:56:05 +0200 Subject: [PATCH 24/41] regenerate --- src/python/grpcio_testing/grpc_version.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/python/grpcio_testing/grpc_version.py b/src/python/grpcio_testing/grpc_version.py index 41a75d46f68..1638b38c2ef 100644 --- a/src/python/grpcio_testing/grpc_version.py +++ b/src/python/grpcio_testing/grpc_version.py @@ -12,6 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -# AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_reflection/grpc_version.py.template`!!! +# AUTO-GENERATED FROM `$REPO_ROOT/templates/src/python/grpcio_testing/grpc_version.py.template`!!! -VERSION = '1.5.0.dev0' +VERSION='1.6.1' From 4984135a78594215b3fbf04e33262b36a99bdb14 Mon Sep 17 00:00:00 2001 From: Justin Burke Date: Thu, 31 Aug 2017 17:42:54 -0700 Subject: [PATCH 25/41] Add reference counting to tsi_ssl_handshaker_factories --- .../lib/http/httpcli_security_connector.c | 3 +- .../security/transport/security_connector.c | 22 ++-- src/core/tsi/ssl_transport_security.c | 113 ++++++++++++++-- src/core/tsi/ssl_transport_security.h | 37 +++++- test/core/tsi/ssl_transport_security_test.c | 121 +++++++++++++++++- 5 files changed, 265 insertions(+), 31 deletions(-) diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c index 97c28865258..c553fa3981e 100644 --- a/src/core/lib/http/httpcli_security_connector.c +++ b/src/core/lib/http/httpcli_security_connector.c @@ -43,7 +43,8 @@ static void httpcli_ssl_destroy(grpc_exec_ctx *exec_ctx, grpc_httpcli_ssl_channel_security_connector *c = (grpc_httpcli_ssl_channel_security_connector *)sc; if (c->handshaker_factory != NULL) { - tsi_ssl_client_handshaker_factory_destroy(c->handshaker_factory); + tsi_ssl_client_handshaker_factory_unref(c->handshaker_factory); + c->handshaker_factory = NULL; } if (c->secure_peer_name != NULL) gpr_free(c->secure_peer_name); gpr_free(sc); diff --git a/src/core/lib/security/transport/security_connector.c b/src/core/lib/security/transport/security_connector.c index a7568b995f5..2a9e939d40a 100644 --- a/src/core/lib/security/transport/security_connector.c +++ b/src/core/lib/security/transport/security_connector.c @@ -455,14 +455,14 @@ grpc_server_security_connector *grpc_fake_server_security_connector_create( typedef struct { grpc_channel_security_connector base; - tsi_ssl_client_handshaker_factory *handshaker_factory; + tsi_ssl_client_handshaker_factory *client_handshaker_factory; char *target_name; char *overridden_target_name; } grpc_ssl_channel_security_connector; typedef struct { grpc_server_security_connector base; - tsi_ssl_server_handshaker_factory *handshaker_factory; + tsi_ssl_server_handshaker_factory *server_handshaker_factory; } grpc_ssl_server_security_connector; static void ssl_channel_destroy(grpc_exec_ctx *exec_ctx, @@ -470,9 +470,8 @@ static void ssl_channel_destroy(grpc_exec_ctx *exec_ctx, grpc_ssl_channel_security_connector *c = (grpc_ssl_channel_security_connector *)sc; grpc_call_credentials_unref(exec_ctx, c->base.request_metadata_creds); - if (c->handshaker_factory != NULL) { - tsi_ssl_client_handshaker_factory_destroy(c->handshaker_factory); - } + tsi_ssl_client_handshaker_factory_unref(c->client_handshaker_factory); + c->client_handshaker_factory = NULL; if (c->target_name != NULL) gpr_free(c->target_name); if (c->overridden_target_name != NULL) gpr_free(c->overridden_target_name); gpr_free(sc); @@ -482,9 +481,8 @@ static void ssl_server_destroy(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc) { grpc_ssl_server_security_connector *c = (grpc_ssl_server_security_connector *)sc; - if (c->handshaker_factory != NULL) { - tsi_ssl_server_handshaker_factory_destroy(c->handshaker_factory); - } + tsi_ssl_server_handshaker_factory_unref(c->server_handshaker_factory); + c->server_handshaker_factory = NULL; gpr_free(sc); } @@ -496,7 +494,7 @@ static void ssl_channel_add_handshakers(grpc_exec_ctx *exec_ctx, // Instantiate TSI handshaker. tsi_handshaker *tsi_hs = NULL; tsi_result result = tsi_ssl_client_handshaker_factory_create_handshaker( - c->handshaker_factory, + c->client_handshaker_factory, c->overridden_target_name != NULL ? c->overridden_target_name : c->target_name, &tsi_hs); @@ -521,7 +519,7 @@ static void ssl_server_add_handshakers(grpc_exec_ctx *exec_ctx, // Instantiate TSI handshaker. tsi_handshaker *tsi_hs = NULL; tsi_result result = tsi_ssl_server_handshaker_factory_create_handshaker( - c->handshaker_factory, &tsi_hs); + c->server_handshaker_factory, &tsi_hs); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", tsi_result_to_string(result)); @@ -852,7 +850,7 @@ grpc_security_status grpc_ssl_channel_security_connector_create( result = tsi_create_ssl_client_handshaker_factory( has_key_cert_pair ? &config->pem_key_cert_pair : NULL, pem_root_certs, ssl_cipher_suites(), alpn_protocol_strings, (uint16_t)num_alpn_protocols, - &c->handshaker_factory); + &c->client_handshaker_factory); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", tsi_result_to_string(result)); @@ -897,7 +895,7 @@ grpc_security_status grpc_ssl_server_security_connector_create( config->pem_root_certs, get_tsi_client_certificate_request_type( config->client_certificate_request), ssl_cipher_suites(), alpn_protocol_strings, (uint16_t)num_alpn_protocols, - &c->handshaker_factory); + &c->server_handshaker_factory); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.", tsi_result_to_string(result)); diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 1fd65928f9c..7ebf9dd96f0 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -67,7 +67,13 @@ /* --- Structure definitions. ---*/ +struct tsi_ssl_handshaker_factory { + const tsi_ssl_handshaker_factory_vtable *vtable; + gpr_refcount refcount; +}; + struct tsi_ssl_client_handshaker_factory { + tsi_ssl_handshaker_factory base; SSL_CTX *ssl_context; unsigned char *alpn_protocol_list; size_t alpn_protocol_list_length; @@ -77,6 +83,7 @@ struct tsi_ssl_server_handshaker_factory { /* Several contexts to support SNI. The tsi_peer array contains the subject names of the server certificates associated with the contexts at the same index. */ + tsi_ssl_handshaker_factory base; SSL_CTX **ssl_contexts; tsi_peer *ssl_context_x509_subject_names; size_t ssl_context_count; @@ -90,6 +97,7 @@ typedef struct { BIO *into_ssl; BIO *from_ssl; tsi_result result; + tsi_ssl_handshaker_factory *factory_ref; } tsi_ssl_handshaker; typedef struct { @@ -846,6 +854,47 @@ static const tsi_frame_protector_vtable frame_protector_vtable = { ssl_protector_destroy, }; +/* --- tsi_server_handshaker_factory methods implementation. --- */ + +static void tsi_ssl_handshaker_factory_destroy( + tsi_ssl_handshaker_factory *self) { + if (self == NULL) return; + + if (self->vtable != NULL && self->vtable->destroy != NULL) { + self->vtable->destroy(self); + } + /* Note, we don't free(self) here because this object is always directly + * embedded in another object. If tsi_ssl_handshaker_factory_init allocates + * any memory, it should be free'd here. */ +} + +static tsi_ssl_handshaker_factory *tsi_ssl_handshaker_factory_ref( + tsi_ssl_handshaker_factory *self) { + if (self == NULL) return NULL; + gpr_refn(&self->refcount, 1); + return self; +} + +static void tsi_ssl_handshaker_factory_unref(tsi_ssl_handshaker_factory *self) { + if (self == NULL) return; + + if (gpr_unref(&self->refcount)) { + tsi_ssl_handshaker_factory_destroy(self); + } +} + +static tsi_ssl_handshaker_factory_vtable handshaker_factory_vtable = {NULL}; + +/* Initializes a tsi_ssl_handshaker_factory object. Caller is responsible for + * allocating memory for the factory. */ +static void tsi_ssl_handshaker_factory_init( + tsi_ssl_handshaker_factory *factory) { + GPR_ASSERT(factory != NULL); + + factory->vtable = &handshaker_factory_vtable; + gpr_ref_init(&factory->refcount, 1); +} + /* --- tsi_handshaker methods implementation. ---*/ static tsi_result ssl_handshaker_get_bytes_to_send_to_peer(tsi_handshaker *self, @@ -1013,6 +1062,7 @@ static tsi_result ssl_handshaker_create_frame_protector( static void ssl_handshaker_destroy(tsi_handshaker *self) { tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; SSL_free(impl->ssl); /* The BIO objects are owned by ssl */ + tsi_ssl_handshaker_factory_unref(impl->factory_ref); gpr_free(impl); } @@ -1030,6 +1080,7 @@ static const tsi_handshaker_vtable handshaker_vtable = { static tsi_result create_tsi_ssl_handshaker(SSL_CTX *ctx, int is_client, const char *server_name_indication, + tsi_ssl_handshaker_factory *factory, tsi_handshaker **handshaker) { SSL *ssl = SSL_new(ctx); BIO *into_ssl = NULL; @@ -1085,6 +1136,8 @@ static tsi_result create_tsi_ssl_handshaker(SSL_CTX *ctx, int is_client, impl->from_ssl = from_ssl; impl->result = TSI_HANDSHAKE_IN_PROGRESS; impl->base.vtable = &handshaker_vtable; + impl->factory_ref = tsi_ssl_handshaker_factory_ref(factory); + *handshaker = &impl->base; return TSI_OK; } @@ -1121,11 +1174,20 @@ tsi_result tsi_ssl_client_handshaker_factory_create_handshaker( tsi_ssl_client_handshaker_factory *self, const char *server_name_indication, tsi_handshaker **handshaker) { return create_tsi_ssl_handshaker(self->ssl_context, 1, server_name_indication, - handshaker); + &self->base, handshaker); } -void tsi_ssl_client_handshaker_factory_destroy( +void tsi_ssl_client_handshaker_factory_unref( tsi_ssl_client_handshaker_factory *self) { + if (self == NULL) return; + tsi_ssl_handshaker_factory_unref(&self->base); +} + +static void tsi_ssl_client_handshaker_factory_destroy( + tsi_ssl_handshaker_factory *factory) { + if (factory == NULL) return; + tsi_ssl_client_handshaker_factory *self = + (tsi_ssl_client_handshaker_factory *)factory; if (self->ssl_context != NULL) SSL_CTX_free(self->ssl_context); if (self->alpn_protocol_list != NULL) gpr_free(self->alpn_protocol_list); gpr_free(self); @@ -1150,11 +1212,21 @@ tsi_result tsi_ssl_server_handshaker_factory_create_handshaker( if (self->ssl_context_count == 0) return TSI_INVALID_ARGUMENT; /* Create the handshaker with the first context. We will switch if needed because of SNI in ssl_server_handshaker_factory_servername_callback. */ - return create_tsi_ssl_handshaker(self->ssl_contexts[0], 0, NULL, handshaker); + return create_tsi_ssl_handshaker(self->ssl_contexts[0], 0, NULL, &self->base, + handshaker); } -void tsi_ssl_server_handshaker_factory_destroy( +void tsi_ssl_server_handshaker_factory_unref( tsi_ssl_server_handshaker_factory *self) { + if (self == NULL) return; + tsi_ssl_handshaker_factory_unref(&self->base); +} + +static void tsi_ssl_server_handshaker_factory_destroy( + tsi_ssl_handshaker_factory *factory) { + if (factory == NULL) return; + tsi_ssl_server_handshaker_factory *self = + (tsi_ssl_server_handshaker_factory *)factory; size_t i; for (i = 0; i < self->ssl_context_count; i++) { if (self->ssl_contexts[i] != NULL) { @@ -1263,6 +1335,9 @@ static int server_handshaker_factory_npn_advertised_callback( /* --- tsi_ssl_handshaker_factory constructors. --- */ +static tsi_ssl_handshaker_factory_vtable client_handshaker_factory_vtable = { + tsi_ssl_client_handshaker_factory_destroy}; + tsi_result tsi_create_ssl_client_handshaker_factory( const tsi_ssl_pem_key_cert_pair *pem_key_cert_pair, const char *pem_root_certs, const char *cipher_suites, @@ -1285,6 +1360,9 @@ tsi_result tsi_create_ssl_client_handshaker_factory( } impl = gpr_zalloc(sizeof(*impl)); + tsi_ssl_handshaker_factory_init(&impl->base); + impl->base.vtable = &client_handshaker_factory_vtable; + impl->ssl_context = ssl_context; do { @@ -1322,7 +1400,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory( } } while (0); if (result != TSI_OK) { - tsi_ssl_client_handshaker_factory_destroy(impl); + tsi_ssl_handshaker_factory_unref(&impl->base); return result; } SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, NULL); @@ -1332,6 +1410,9 @@ tsi_result tsi_create_ssl_client_handshaker_factory( return TSI_OK; } +static tsi_ssl_handshaker_factory_vtable server_handshaker_factory_vtable = { + tsi_ssl_server_handshaker_factory_destroy}; + tsi_result tsi_create_ssl_server_handshaker_factory( const tsi_ssl_pem_key_cert_pair *pem_key_cert_pairs, size_t num_key_cert_pairs, const char *pem_client_root_certs, @@ -1364,12 +1445,15 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( } impl = gpr_zalloc(sizeof(*impl)); + tsi_ssl_handshaker_factory_init(&impl->base); + impl->base.vtable = &server_handshaker_factory_vtable; + impl->ssl_contexts = gpr_zalloc(num_key_cert_pairs * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = gpr_zalloc(num_key_cert_pairs * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { - tsi_ssl_server_handshaker_factory_destroy(impl); + tsi_ssl_handshaker_factory_unref(&impl->base); return TSI_OUT_OF_RESOURCES; } impl->ssl_context_count = num_key_cert_pairs; @@ -1379,7 +1463,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( &impl->alpn_protocol_list, &impl->alpn_protocol_list_length); if (result != TSI_OK) { - tsi_ssl_server_handshaker_factory_destroy(impl); + tsi_ssl_handshaker_factory_unref(&impl->base); return result; } } @@ -1451,10 +1535,11 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( } while (0); if (result != TSI_OK) { - tsi_ssl_server_handshaker_factory_destroy(impl); + tsi_ssl_handshaker_factory_unref(&impl->base); return result; } } + *factory = impl; return TSI_OK; } @@ -1501,3 +1586,15 @@ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name) { return 0; /* Not found. */ } + +/* --- Testing support. --- */ +const tsi_ssl_handshaker_factory_vtable *tsi_ssl_handshaker_factory_swap_vtable( + tsi_ssl_handshaker_factory *factory, + tsi_ssl_handshaker_factory_vtable *new_vtable) { + GPR_ASSERT(factory != NULL); + GPR_ASSERT(factory->vtable != NULL); + + const tsi_ssl_handshaker_factory_vtable *orig_vtable = factory->vtable; + factory->vtable = new_vtable; + return orig_vtable; +} diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index 177599930bc..3abfdf5ed87 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -96,10 +96,10 @@ tsi_result tsi_ssl_client_handshaker_factory_create_handshaker( tsi_ssl_client_handshaker_factory *self, const char *server_name_indication, tsi_handshaker **handshaker); -/* Destroys the handshaker factory. WARNING: it is unsafe to destroy a factory - while handshakers created with this factory are still in use. */ -void tsi_ssl_client_handshaker_factory_destroy( - tsi_ssl_client_handshaker_factory *self); +/* Decrements reference count of the handshaker factory. Handshaker factory will + * be destroyed once no references exist. */ +void tsi_ssl_client_handshaker_factory_unref( + tsi_ssl_client_handshaker_factory *factory); /* --- tsi_ssl_server_handshaker_factory object --- @@ -158,9 +158,9 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( tsi_result tsi_ssl_server_handshaker_factory_create_handshaker( tsi_ssl_server_handshaker_factory *self, tsi_handshaker **handshaker); -/* Destroys the handshaker factory. WARNING: it is unsafe to destroy a factory - while handshakers created with this factory are still in use. */ -void tsi_ssl_server_handshaker_factory_destroy( +/* Decrements reference count of the handshaker factory. Handshaker factory will + * be destroyed once no references exist. */ +void tsi_ssl_server_handshaker_factory_unref( tsi_ssl_server_handshaker_factory *self); /* Util that checks that an ssl peer matches a specific name. @@ -170,6 +170,29 @@ void tsi_ssl_server_handshaker_factory_destroy( - handle public suffix wildchar more strictly (e.g. *.co.uk) */ int tsi_ssl_peer_matches_name(const tsi_peer *peer, const char *name); +/* --- Testing support. --- + + These functions and typedefs are not intended to be used outside of testing. + */ + +/* Base type of client and server handshaker factories. */ +typedef struct tsi_ssl_handshaker_factory tsi_ssl_handshaker_factory; + +/* Function pointer to handshaker_factory destructor. */ +typedef void (*tsi_ssl_handshaker_factory_destructor)( + tsi_ssl_handshaker_factory *factory); + +/* Virtual table for tsi_ssl_handshaker_factory. */ +typedef struct { + tsi_ssl_handshaker_factory_destructor destroy; +} tsi_ssl_handshaker_factory_vtable; + +/* Set destructor of handshaker_factory to new_destructor, returns previous + destructor. */ +const tsi_ssl_handshaker_factory_vtable *tsi_ssl_handshaker_factory_swap_vtable( + tsi_ssl_handshaker_factory *factory, + tsi_ssl_handshaker_factory_vtable *new_vtable); + #ifdef __cplusplus } #endif diff --git a/test/core/tsi/ssl_transport_security_test.c b/test/core/tsi/ssl_transport_security_test.c index 364dfa1b73f..2399b054b1b 100644 --- a/test/core/tsi/ssl_transport_security_test.c +++ b/test/core/tsi/ssl_transport_security_test.c @@ -23,7 +23,9 @@ #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/security/transport/security_connector.h" #include "src/core/tsi/ssl_transport_security.h" +#include "src/core/tsi/transport_security.h" #include "src/core/tsi/transport_security_adapter.h" +#include "src/core/tsi/transport_security_interface.h" #include "test/core/tsi/transport_security_test_lib.h" #include "test/core/util/test_config.h" @@ -312,10 +314,10 @@ static void ssl_test_destruct(tsi_test_fixture *fixture) { key_cert_lib->bad_client_pem_key_cert_pair); gpr_free(key_cert_lib->root_cert); gpr_free(key_cert_lib); - /* Destroy others. */ - tsi_ssl_server_handshaker_factory_destroy( + /* Unreference others. */ + tsi_ssl_server_handshaker_factory_unref( ssl_fixture->server_handshaker_factory); - tsi_ssl_client_handshaker_factory_destroy( + tsi_ssl_client_handshaker_factory_unref( ssl_fixture->client_handshaker_factory); } @@ -536,6 +538,118 @@ void ssl_tsi_test_do_round_trip_odd_buffer_size() { } } +static const tsi_ssl_handshaker_factory_vtable *original_vtable; +static bool handshaker_factory_destructor_called; + +static void ssl_tsi_test_handshaker_factory_destructor( + tsi_ssl_handshaker_factory *factory) { + GPR_ASSERT(factory != NULL); + handshaker_factory_destructor_called = true; + if (original_vtable != NULL && original_vtable->destroy != NULL) { + original_vtable->destroy(factory); + } +} + +static tsi_ssl_handshaker_factory_vtable test_handshaker_factory_vtable = { + ssl_tsi_test_handshaker_factory_destructor}; + +void test_tsi_ssl_client_handshaker_factory_refcounting() { + int i; + const char *cert_chain = + load_file(SSL_TSI_TEST_CREDENTIALS_DIR, "client.pem"); + + tsi_ssl_client_handshaker_factory *client_handshaker_factory; + GPR_ASSERT(tsi_create_ssl_client_handshaker_factory( + NULL, cert_chain, NULL, NULL, 0, &client_handshaker_factory) == + TSI_OK); + + handshaker_factory_destructor_called = false; + original_vtable = tsi_ssl_handshaker_factory_swap_vtable( + (tsi_ssl_handshaker_factory *)client_handshaker_factory, + &test_handshaker_factory_vtable); + + tsi_handshaker *handshaker[3]; + + for (i = 0; i < 3; ++i) { + GPR_ASSERT(tsi_ssl_client_handshaker_factory_create_handshaker( + client_handshaker_factory, "google.com", &handshaker[i]) == + TSI_OK); + } + + tsi_handshaker_destroy(handshaker[1]); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_handshaker_destroy(handshaker[0]); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_ssl_client_handshaker_factory_unref(client_handshaker_factory); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_handshaker_destroy(handshaker[2]); + GPR_ASSERT(handshaker_factory_destructor_called); + + gpr_free((void *)cert_chain); +} + +void test_tsi_ssl_server_handshaker_factory_refcounting() { + int i; + tsi_ssl_server_handshaker_factory *server_handshaker_factory; + tsi_handshaker *handshaker[3]; + const char *cert_chain = + load_file(SSL_TSI_TEST_CREDENTIALS_DIR, "server0.pem"); + tsi_ssl_pem_key_cert_pair cert_pair; + + cert_pair.cert_chain = cert_chain; + cert_pair.private_key = + load_file(SSL_TSI_TEST_CREDENTIALS_DIR, "server0.key"); + + GPR_ASSERT(tsi_create_ssl_server_handshaker_factory( + &cert_pair, 1, cert_chain, 0, NULL, NULL, 0, + &server_handshaker_factory) == TSI_OK); + + handshaker_factory_destructor_called = false; + original_vtable = tsi_ssl_handshaker_factory_swap_vtable( + (tsi_ssl_handshaker_factory *)server_handshaker_factory, + &test_handshaker_factory_vtable); + + for (i = 0; i < 3; ++i) { + GPR_ASSERT(tsi_ssl_server_handshaker_factory_create_handshaker( + server_handshaker_factory, &handshaker[i]) == TSI_OK); + } + + tsi_handshaker_destroy(handshaker[1]); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_handshaker_destroy(handshaker[0]); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_ssl_server_handshaker_factory_unref(server_handshaker_factory); + GPR_ASSERT(!handshaker_factory_destructor_called); + + tsi_handshaker_destroy(handshaker[2]); + GPR_ASSERT(handshaker_factory_destructor_called); + + ssl_test_pem_key_cert_pair_destroy(cert_pair); +} + +/* Attempting to create a handshaker factory with invalid parameters should fail + * but not crash. */ +void test_tsi_ssl_client_handshaker_factory_bad_params() { + const char *cert_chain = "This is not a valid PEM file."; + + tsi_ssl_client_handshaker_factory *client_handshaker_factory; + GPR_ASSERT(tsi_create_ssl_client_handshaker_factory( + NULL, cert_chain, NULL, NULL, 0, &client_handshaker_factory) == + TSI_INVALID_ARGUMENT); + tsi_ssl_client_handshaker_factory_unref(client_handshaker_factory); +} + +void ssl_tsi_test_handshaker_factory_internals() { + test_tsi_ssl_client_handshaker_factory_refcounting(); + test_tsi_ssl_server_handshaker_factory_refcounting(); + test_tsi_ssl_client_handshaker_factory_bad_params(); +} + int main(int argc, char **argv) { grpc_test_init(argc, argv); grpc_init(); @@ -553,6 +667,7 @@ int main(int argc, char **argv) { ssl_tsi_test_do_handshake_alpn_client_server_ok(); ssl_tsi_test_do_round_trip_for_all_configs(); ssl_tsi_test_do_round_trip_odd_buffer_size(); + ssl_tsi_test_handshaker_factory_internals(); grpc_shutdown(); return 0; } From 4478a88ad92562a315267cfea8e6f3a29d1da654 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Fri, 15 Sep 2017 14:25:35 -0700 Subject: [PATCH 26/41] More detailed error when double-scheduling a closure --- src/core/lib/iomgr/closure.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c index 7236e23cf70..2253604342e 100644 --- a/src/core/lib/iomgr/closure.c +++ b/src/core/lib/iomgr/closure.c @@ -167,7 +167,15 @@ void grpc_closure_sched(grpc_exec_ctx *exec_ctx, grpc_closure *c, GPR_TIMER_BEGIN("grpc_closure_sched", 0); if (c != NULL) { #ifndef NDEBUG - GPR_ASSERT(!c->scheduled); + if (c->scheduled) { + gpr_log(GPR_ERROR, + "Closure already scheduled. (closure: %p, created: [%s:%d], " + "previously scheduled at: [%s: %d] run?: %s", + closure, closure->file_created, closure->line_created, + closure->file_initiated, closure->line_initiated, + closure->run ? "true" : "false"); + abort(); + } c->scheduled = true; c->file_initiated = file; c->line_initiated = line; From 571fa0c5967a4e7a504e5ad5c495bb4a68953166 Mon Sep 17 00:00:00 2001 From: ncteisen Date: Mon, 11 Sep 2017 16:06:11 -0700 Subject: [PATCH 27/41] Add stream state tracer --- .../chttp2/transport/chttp2_plugin.c | 1 + .../chttp2/transport/chttp2_transport.h | 1 + .../transport/chttp2/transport/stream_lists.c | 52 +++++++++++++------ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_plugin.c b/src/core/ext/transport/chttp2/transport/chttp2_plugin.c index 78551df9c3c..6d09953830c 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_plugin.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_plugin.c @@ -23,6 +23,7 @@ void grpc_chttp2_plugin_init(void) { grpc_register_tracer(&grpc_http_trace); grpc_register_tracer(&grpc_flowctl_trace); + grpc_register_tracer(&grpc_trace_http2_stream_state); #ifndef NDEBUG grpc_register_tracer(&grpc_trace_chttp2_refcount); #endif diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.h b/src/core/ext/transport/chttp2/transport/chttp2_transport.h index 0c4e2a91c03..55fb1a8343e 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.h +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.h @@ -25,6 +25,7 @@ extern grpc_tracer_flag grpc_http_trace; extern grpc_tracer_flag grpc_flowctl_trace; +extern grpc_tracer_flag grpc_trace_http2_stream_state; #ifndef NDEBUG extern grpc_tracer_flag grpc_trace_chttp2_refcount; diff --git a/src/core/ext/transport/chttp2/transport/stream_lists.c b/src/core/ext/transport/chttp2/transport/stream_lists.c index 7cc85dea9c8..ee23b77efc5 100644 --- a/src/core/ext/transport/chttp2/transport/stream_lists.c +++ b/src/core/ext/transport/chttp2/transport/stream_lists.c @@ -20,6 +20,27 @@ #include +static char *stream_list_id_string(grpc_chttp2_stream_list_id id) { + switch (id) { + case GRPC_CHTTP2_LIST_WRITABLE: + return "writable"; + case GRPC_CHTTP2_LIST_WRITING: + return "writing"; + case GRPC_CHTTP2_LIST_STALLED_BY_TRANSPORT: + return "stalled_by_transport"; + case GRPC_CHTTP2_LIST_STALLED_BY_STREAM: + return "stalled_by_stream"; + case GRPC_CHTTP2_LIST_WAITING_FOR_CONCURRENCY: + return "waiting_for_concurrency"; + case STREAM_LIST_COUNT: + GPR_UNREACHABLE_CODE(return "unknown"); + } + GPR_UNREACHABLE_CODE(return "unknown"); +} + +grpc_tracer_flag grpc_trace_http2_stream_state = + GRPC_TRACER_INITIALIZER(false, "http2_stream_state"); + /* core list management */ static bool stream_list_empty(grpc_chttp2_transport *t, @@ -44,6 +65,10 @@ static bool stream_list_pop(grpc_chttp2_transport *t, s->included[id] = 0; } *stream = s; + if (s && GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { + gpr_log(GPR_DEBUG, "t[%p]: pop stream %d from %s", t, s->id, + stream_list_id_string(id)); + } return s != 0; } @@ -62,6 +87,10 @@ static void stream_list_remove(grpc_chttp2_transport *t, grpc_chttp2_stream *s, } else { t->lists[id].tail = s->links[id].prev; } + if (GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { + gpr_log(GPR_DEBUG, "t[%p]: remove stream %d from %s", t, s->id, + stream_list_id_string(id)); + } } static bool stream_list_maybe_remove(grpc_chttp2_transport *t, @@ -90,6 +119,10 @@ static void stream_list_add_tail(grpc_chttp2_transport *t, } t->lists[id].tail = s; s->included[id] = 1; + if (GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { + gpr_log(GPR_DEBUG, "t[%p]: add stream %d from %s", t, s->id, + stream_list_id_string(id)); + } } static bool stream_list_add(grpc_chttp2_transport *t, grpc_chttp2_stream *s, @@ -150,17 +183,12 @@ void grpc_chttp2_list_remove_waiting_for_concurrency(grpc_chttp2_transport *t, void grpc_chttp2_list_add_stalled_by_transport(grpc_chttp2_transport *t, grpc_chttp2_stream *s) { - GRPC_FLOW_CONTROL_IF_TRACING( - gpr_log(GPR_DEBUG, "stream %u stalled by transport", s->id)); stream_list_add(t, s, GRPC_CHTTP2_LIST_STALLED_BY_TRANSPORT); } bool grpc_chttp2_list_pop_stalled_by_transport(grpc_chttp2_transport *t, grpc_chttp2_stream **s) { - bool ret = stream_list_pop(t, s, GRPC_CHTTP2_LIST_STALLED_BY_TRANSPORT); - GRPC_FLOW_CONTROL_IF_TRACING(if (ret) gpr_log( - GPR_DEBUG, "stream %u un-stalled by transport", (*s)->id)); - return ret; + return stream_list_pop(t, s, GRPC_CHTTP2_LIST_STALLED_BY_TRANSPORT); } void grpc_chttp2_list_remove_stalled_by_transport(grpc_chttp2_transport *t, @@ -170,23 +198,15 @@ void grpc_chttp2_list_remove_stalled_by_transport(grpc_chttp2_transport *t, void grpc_chttp2_list_add_stalled_by_stream(grpc_chttp2_transport *t, grpc_chttp2_stream *s) { - GRPC_FLOW_CONTROL_IF_TRACING( - gpr_log(GPR_DEBUG, "stream %u stalled by stream", s->id)); stream_list_add(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM); } bool grpc_chttp2_list_pop_stalled_by_stream(grpc_chttp2_transport *t, grpc_chttp2_stream **s) { - bool ret = stream_list_pop(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM); - GRPC_FLOW_CONTROL_IF_TRACING( - if (ret) gpr_log(GPR_DEBUG, "stream %u un-stalled by stream", (*s)->id)); - return ret; + return stream_list_pop(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM); } bool grpc_chttp2_list_remove_stalled_by_stream(grpc_chttp2_transport *t, grpc_chttp2_stream *s) { - bool ret = stream_list_maybe_remove(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM); - GRPC_FLOW_CONTROL_IF_TRACING( - if (ret) gpr_log(GPR_DEBUG, "stream %u un-stalled by stream", s->id)); - return ret; + return stream_list_maybe_remove(t, s, GRPC_CHTTP2_LIST_STALLED_BY_STREAM); } From 0252252fdd5c468079b3d5e32cc4516b02dccbcc Mon Sep 17 00:00:00 2001 From: ncteisen Date: Mon, 11 Sep 2017 18:01:23 -0700 Subject: [PATCH 28/41] Update env var documentation --- doc/environment_variables.md | 1 + .../ext/transport/chttp2/transport/stream_lists.c | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/environment_variables.md b/doc/environment_variables.md index b79cd973633..f90f1d5b10e 100644 --- a/doc/environment_variables.md +++ b/doc/environment_variables.md @@ -50,6 +50,7 @@ some configuration as environment variables that can be set. - channel_stack_builder - traces information about channel stacks being built - executor - traces grpc's internal thread pool ('the executor') - http - traces state in the http2 transport engine + - http2_stream_state - traces all http2 stream state mutations. - http1 - traces HTTP/1.x operations performed by gRPC - inproc - traces the in-process transport - flowctl - traces http2 flow control diff --git a/src/core/ext/transport/chttp2/transport/stream_lists.c b/src/core/ext/transport/chttp2/transport/stream_lists.c index ee23b77efc5..47cd22d1773 100644 --- a/src/core/ext/transport/chttp2/transport/stream_lists.c +++ b/src/core/ext/transport/chttp2/transport/stream_lists.c @@ -66,8 +66,8 @@ static bool stream_list_pop(grpc_chttp2_transport *t, } *stream = s; if (s && GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { - gpr_log(GPR_DEBUG, "t[%p]: pop stream %d from %s", t, s->id, - stream_list_id_string(id)); + gpr_log(GPR_DEBUG, "%p[%d][%s]: pop from %s", t, s->id, + t->is_client ? "cli" : "svr", stream_list_id_string(id)); } return s != 0; } @@ -88,8 +88,8 @@ static void stream_list_remove(grpc_chttp2_transport *t, grpc_chttp2_stream *s, t->lists[id].tail = s->links[id].prev; } if (GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { - gpr_log(GPR_DEBUG, "t[%p]: remove stream %d from %s", t, s->id, - stream_list_id_string(id)); + gpr_log(GPR_DEBUG, "%p[%d][%s]: remove from %s", t, s->id, + t->is_client ? "cli" : "svr", stream_list_id_string(id)); } } @@ -120,8 +120,8 @@ static void stream_list_add_tail(grpc_chttp2_transport *t, t->lists[id].tail = s; s->included[id] = 1; if (GRPC_TRACER_ON(grpc_trace_http2_stream_state)) { - gpr_log(GPR_DEBUG, "t[%p]: add stream %d from %s", t, s->id, - stream_list_id_string(id)); + gpr_log(GPR_DEBUG, "%p[%d][%s]: add to %s", t, s->id, + t->is_client ? "cli" : "svr", stream_list_id_string(id)); } } From 966bf49b9c7b916c4f4a2be1dc7a9e8dae07582d Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 18 Sep 2017 09:42:31 -0700 Subject: [PATCH 29/41] Add to closure_list_sched as well --- src/core/lib/iomgr/closure.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c index 2253604342e..bb9e437b08f 100644 --- a/src/core/lib/iomgr/closure.c +++ b/src/core/lib/iomgr/closure.c @@ -199,7 +199,15 @@ void grpc_closure_list_sched(grpc_exec_ctx *exec_ctx, grpc_closure_list *list) { while (c != NULL) { grpc_closure *next = c->next_data.next; #ifndef NDEBUG - GPR_ASSERT(!c->scheduled); + if (c->scheduled) { + gpr_log(GPR_ERROR, + "Closure already scheduled. (closure: %p, created: [%s:%d], " + "previously scheduled at: [%s: %d] run?: %s", + closure, closure->file_created, closure->line_created, + closure->file_initiated, closure->line_initiated, + closure->run ? "true" : "false"); + abort(); + } c->scheduled = true; c->file_initiated = file; c->line_initiated = line; From 086e95ae657ab87b52de9675140b194c61874683 Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Sun, 17 Sep 2017 23:59:11 +0000 Subject: [PATCH 30/41] Use grpc_1_0 flag in beta_python_plugin_test Beta code elements are not generated at all in _pb2_grpc.py files. This duplicates a lot of the in-test code generation done in _split_definitions_test. In a future clean-up we may want to deduplicate the common behavior, put it in a module available to all other tests, and do all of our testing of generated code with in-test code generation. --- .../protoc_plugin/beta_python_plugin_test.py | 341 +++++++++++++----- 1 file changed, 256 insertions(+), 85 deletions(-) diff --git a/src/python/grpcio_tests/tests/protoc_plugin/beta_python_plugin_test.py b/src/python/grpcio_tests/tests/protoc_plugin/beta_python_plugin_test.py index 83f21ecbbb7..424b153ff8d 100644 --- a/src/python/grpcio_tests/tests/protoc_plugin/beta_python_plugin_test.py +++ b/src/python/grpcio_tests/tests/protoc_plugin/beta_python_plugin_test.py @@ -12,19 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -import argparse import contextlib -import distutils.spawn -import errno -import itertools +import importlib import os -import pkg_resources +from os import path +import pkgutil import shutil -import subprocess import sys import tempfile import threading -import time import unittest from six import moves @@ -33,12 +29,22 @@ from grpc.beta import implementations from grpc.beta import interfaces from grpc.framework.foundation import future from grpc.framework.interfaces.face import face +from grpc_tools import protoc from tests.unit.framework.common import test_constants -import tests.protoc_plugin.protos.payload.test_payload_pb2 as payload_pb2 -import tests.protoc_plugin.protos.requests.r.test_requests_pb2 as request_pb2 -import tests.protoc_plugin.protos.responses.test_responses_pb2 as response_pb2 -import tests.protoc_plugin.protos.service.test_service_pb2 as service_pb2 +_RELATIVE_PROTO_PATH = 'relative_proto_path' +_RELATIVE_PYTHON_OUT = 'relative_python_out' + +_PROTO_FILES_PATH_COMPONENTS = ( + ('beta_grpc_plugin_test', 'payload', 'test_payload.proto',), + ('beta_grpc_plugin_test', 'requests', 'r', 'test_requests.proto',), + ('beta_grpc_plugin_test', 'responses', 'test_responses.proto',), + ('beta_grpc_plugin_test', 'service', 'test_service.proto',),) + +_PAYLOAD_PB2 = 'beta_grpc_plugin_test.payload.test_payload_pb2' +_REQUESTS_PB2 = 'beta_grpc_plugin_test.requests.r.test_requests_pb2' +_RESPONSES_PB2 = 'beta_grpc_plugin_test.responses.test_responses_pb2' +_SERVICE_PB2 = 'beta_grpc_plugin_test.service.test_service_pb2' # Identifiers of entities we expect to find in the generated module. SERVICER_IDENTIFIER = 'BetaTestServiceServicer' @@ -47,12 +53,50 @@ SERVER_FACTORY_IDENTIFIER = 'beta_create_TestService_server' STUB_FACTORY_IDENTIFIER = 'beta_create_TestService_stub' +@contextlib.contextmanager +def _system_path(path_insertion): + old_system_path = sys.path[:] + sys.path = sys.path[0:1] + path_insertion + sys.path[1:] + yield + sys.path = old_system_path + + +def _create_directory_tree(root, path_components_sequence): + created = set() + for path_components in path_components_sequence: + thus_far = '' + for path_component in path_components: + relative_path = path.join(thus_far, path_component) + if relative_path not in created: + os.makedirs(path.join(root, relative_path)) + created.add(relative_path) + thus_far = path.join(thus_far, path_component) + + +def _massage_proto_content(raw_proto_content): + imports_substituted = raw_proto_content.replace( + b'import "tests/protoc_plugin/protos/', + b'import "beta_grpc_plugin_test/') + package_statement_substituted = imports_substituted.replace( + b'package grpc_protoc_plugin;', b'package beta_grpc_protoc_plugin;') + return package_statement_substituted + + +def _packagify(directory): + for subdirectory, _, _ in os.walk(directory): + init_file_name = path.join(subdirectory, '__init__.py') + with open(init_file_name, 'wb') as init_file: + init_file.write(b'') + + class _ServicerMethods(object): - def __init__(self): + def __init__(self, payload_pb2, responses_pb2): self._condition = threading.Condition() self._paused = False self._fail = False + self._payload_pb2 = payload_pb2 + self._responses_pb2 = responses_pb2 @contextlib.contextmanager def pause(self): # pylint: disable=invalid-name @@ -79,22 +123,22 @@ class _ServicerMethods(object): self._condition.wait() def UnaryCall(self, request, unused_rpc_context): - response = response_pb2.SimpleResponse() - response.payload.payload_type = payload_pb2.COMPRESSABLE + response = self._responses_pb2.SimpleResponse() + response.payload.payload_type = self._payload_pb2.COMPRESSABLE response.payload.payload_compressable = 'a' * request.response_size self._control() return response def StreamingOutputCall(self, request, unused_rpc_context): for parameter in request.response_parameters: - response = response_pb2.StreamingOutputCallResponse() - response.payload.payload_type = payload_pb2.COMPRESSABLE + response = self._responses_pb2.StreamingOutputCallResponse() + response.payload.payload_type = self._payload_pb2.COMPRESSABLE response.payload.payload_compressable = 'a' * parameter.size self._control() yield response def StreamingInputCall(self, request_iter, unused_rpc_context): - response = response_pb2.StreamingInputCallResponse() + response = self._responses_pb2.StreamingInputCallResponse() aggregated_payload_size = 0 for request in request_iter: aggregated_payload_size += len(request.payload.payload_compressable) @@ -105,8 +149,8 @@ class _ServicerMethods(object): def FullDuplexCall(self, request_iter, unused_rpc_context): for request in request_iter: for parameter in request.response_parameters: - response = response_pb2.StreamingOutputCallResponse() - response.payload.payload_type = payload_pb2.COMPRESSABLE + response = self._responses_pb2.StreamingOutputCallResponse() + response.payload.payload_type = self._payload_pb2.COMPRESSABLE response.payload.payload_compressable = 'a' * parameter.size self._control() yield response @@ -115,8 +159,8 @@ class _ServicerMethods(object): responses = [] for request in request_iter: for parameter in request.response_parameters: - response = response_pb2.StreamingOutputCallResponse() - response.payload.payload_type = payload_pb2.COMPRESSABLE + response = self._responses_pb2.StreamingOutputCallResponse() + response.payload.payload_type = self._payload_pb2.COMPRESSABLE response.payload.payload_compressable = 'a' * parameter.size self._control() responses.append(response) @@ -125,7 +169,7 @@ class _ServicerMethods(object): @contextlib.contextmanager -def _CreateService(): +def _CreateService(payload_pb2, responses_pb2, service_pb2): """Provides a servicer backend and a stub. The servicer is just the implementation of the actual servicer passed to the @@ -136,7 +180,7 @@ def _CreateService(): the service bound to the stub and and stub is the stub on which to invoke RPCs. """ - servicer_methods = _ServicerMethods() + servicer_methods = _ServicerMethods(payload_pb2, responses_pb2) class Servicer(getattr(service_pb2, SERVICER_IDENTIFIER)): @@ -161,12 +205,12 @@ def _CreateService(): server.start() channel = implementations.insecure_channel('localhost', port) stub = getattr(service_pb2, STUB_FACTORY_IDENTIFIER)(channel) - yield (servicer_methods, stub) + yield servicer_methods, stub, server.stop(0) @contextlib.contextmanager -def _CreateIncompleteService(): +def _CreateIncompleteService(service_pb2): """Provides a servicer backend that fails to implement methods and its stub. The servicer is just the implementation of the actual servicer passed to the @@ -192,16 +236,16 @@ def _CreateIncompleteService(): server.stop(0) -def _streaming_input_request_iterator(): +def _streaming_input_request_iterator(payload_pb2, requests_pb2): for _ in range(3): - request = request_pb2.StreamingInputCallRequest() + request = requests_pb2.StreamingInputCallRequest() request.payload.payload_type = payload_pb2.COMPRESSABLE request.payload.payload_compressable = 'a' yield request -def _streaming_output_request(): - request = request_pb2.StreamingOutputCallRequest() +def _streaming_output_request(requests_pb2): + request = requests_pb2.StreamingOutputCallRequest() sizes = [1, 2, 3] request.response_parameters.add(size=sizes[0], interval_us=0) request.response_parameters.add(size=sizes[1], interval_us=0) @@ -209,11 +253,11 @@ def _streaming_output_request(): return request -def _full_duplex_request_iterator(): - request = request_pb2.StreamingOutputCallRequest() +def _full_duplex_request_iterator(requests_pb2): + request = requests_pb2.StreamingOutputCallRequest() request.response_parameters.add(size=1, interval_us=0) yield request - request = request_pb2.StreamingOutputCallRequest() + request = requests_pb2.StreamingOutputCallRequest() request.response_parameters.add(size=2, interval_us=0) request.response_parameters.add(size=3, interval_us=0) yield request @@ -227,22 +271,78 @@ class PythonPluginTest(unittest.TestCase): methods and does not exist for response-streaming methods. """ + def setUp(self): + self._directory = tempfile.mkdtemp(dir='.') + self._proto_path = path.join(self._directory, _RELATIVE_PROTO_PATH) + self._python_out = path.join(self._directory, _RELATIVE_PYTHON_OUT) + + os.makedirs(self._proto_path) + os.makedirs(self._python_out) + + directories_path_components = { + proto_file_path_components[:-1] + for proto_file_path_components in _PROTO_FILES_PATH_COMPONENTS + } + _create_directory_tree(self._proto_path, directories_path_components) + self._proto_file_names = set() + for proto_file_path_components in _PROTO_FILES_PATH_COMPONENTS: + raw_proto_content = pkgutil.get_data( + 'tests.protoc_plugin.protos', + path.join(*proto_file_path_components[1:])) + massaged_proto_content = _massage_proto_content(raw_proto_content) + proto_file_name = path.join(self._proto_path, + *proto_file_path_components) + with open(proto_file_name, 'wb') as proto_file: + proto_file.write(massaged_proto_content) + self._proto_file_names.add(proto_file_name) + + def tearDown(self): + shutil.rmtree(self._directory) + + def _protoc(self): + args = [ + '', + '--proto_path={}'.format(self._proto_path), + '--python_out={}'.format(self._python_out), + '--grpc_python_out=grpc_1_0:{}'.format(self._python_out), + ] + list(self._proto_file_names) + protoc_exit_code = protoc.main(args) + self.assertEqual(0, protoc_exit_code) + + _packagify(self._python_out) + + with _system_path([ + self._python_out, + ]): + self._payload_pb2 = importlib.import_module(_PAYLOAD_PB2) + self._requests_pb2 = importlib.import_module(_REQUESTS_PB2) + self._responses_pb2 = importlib.import_module(_RESPONSES_PB2) + self._service_pb2 = importlib.import_module(_SERVICE_PB2) + def testImportAttributes(self): + self._protoc() + # check that we can access the generated module and its members. - self.assertIsNotNone(getattr(service_pb2, SERVICER_IDENTIFIER, None)) - self.assertIsNotNone(getattr(service_pb2, STUB_IDENTIFIER, None)) self.assertIsNotNone( - getattr(service_pb2, SERVER_FACTORY_IDENTIFIER, None)) + getattr(self._service_pb2, SERVICER_IDENTIFIER, None)) + self.assertIsNotNone(getattr(self._service_pb2, STUB_IDENTIFIER, None)) self.assertIsNotNone( - getattr(service_pb2, STUB_FACTORY_IDENTIFIER, None)) + getattr(self._service_pb2, SERVER_FACTORY_IDENTIFIER, None)) + self.assertIsNotNone( + getattr(self._service_pb2, STUB_FACTORY_IDENTIFIER, None)) def testUpDown(self): - with _CreateService(): - request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2): + self._requests_pb2.SimpleRequest(response_size=13) def testIncompleteServicer(self): - with _CreateIncompleteService() as (_, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateIncompleteService(self._service_pb2) as (_, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) try: stub.UnaryCall(request, test_constants.LONG_TIMEOUT) except face.AbortionError as error: @@ -250,15 +350,21 @@ class PythonPluginTest(unittest.TestCase): error.code) def testUnaryCall(self): - with _CreateService() as (methods, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) response = stub.UnaryCall(request, test_constants.LONG_TIMEOUT) expected_response = methods.UnaryCall(request, 'not a real context!') self.assertEqual(expected_response, response) def testUnaryCallFuture(self): - with _CreateService() as (methods, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) # Check that the call does not block waiting for the server to respond. with methods.pause(): response_future = stub.UnaryCall.future( @@ -268,8 +374,11 @@ class PythonPluginTest(unittest.TestCase): self.assertEqual(expected_response, response) def testUnaryCallFutureExpired(self): - with _CreateService() as (methods, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) with methods.pause(): response_future = stub.UnaryCall.future( request, test_constants.SHORT_TIMEOUT) @@ -277,24 +386,33 @@ class PythonPluginTest(unittest.TestCase): response_future.result() def testUnaryCallFutureCancelled(self): - with _CreateService() as (methods, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) with methods.pause(): response_future = stub.UnaryCall.future(request, 1) response_future.cancel() self.assertTrue(response_future.cancelled()) def testUnaryCallFutureFailed(self): - with _CreateService() as (methods, stub): - request = request_pb2.SimpleRequest(response_size=13) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = self._requests_pb2.SimpleRequest(response_size=13) with methods.fail(): response_future = stub.UnaryCall.future( request, test_constants.LONG_TIMEOUT) self.assertIsNotNone(response_future.exception()) def testStreamingOutputCall(self): - with _CreateService() as (methods, stub): - request = _streaming_output_request() + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = _streaming_output_request(self._requests_pb2) responses = stub.StreamingOutputCall(request, test_constants.LONG_TIMEOUT) expected_responses = methods.StreamingOutputCall( @@ -304,8 +422,11 @@ class PythonPluginTest(unittest.TestCase): self.assertEqual(expected_response, response) def testStreamingOutputCallExpired(self): - with _CreateService() as (methods, stub): - request = _streaming_output_request() + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = _streaming_output_request(self._requests_pb2) with methods.pause(): responses = stub.StreamingOutputCall( request, test_constants.SHORT_TIMEOUT) @@ -313,8 +434,11 @@ class PythonPluginTest(unittest.TestCase): list(responses) def testStreamingOutputCallCancelled(self): - with _CreateService() as (methods, stub): - request = _streaming_output_request() + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = _streaming_output_request(self._requests_pb2) responses = stub.StreamingOutputCall(request, test_constants.LONG_TIMEOUT) next(responses) @@ -323,8 +447,11 @@ class PythonPluginTest(unittest.TestCase): next(responses) def testStreamingOutputCallFailed(self): - with _CreateService() as (methods, stub): - request = _streaming_output_request() + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request = _streaming_output_request(self._requests_pb2) with methods.fail(): responses = stub.StreamingOutputCall(request, 1) self.assertIsNotNone(responses) @@ -332,30 +459,46 @@ class PythonPluginTest(unittest.TestCase): next(responses) def testStreamingInputCall(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): response = stub.StreamingInputCall( - _streaming_input_request_iterator(), + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), test_constants.LONG_TIMEOUT) expected_response = methods.StreamingInputCall( - _streaming_input_request_iterator(), 'not a real RpcContext!') + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), + 'not a real RpcContext!') self.assertEqual(expected_response, response) def testStreamingInputCallFuture(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.pause(): response_future = stub.StreamingInputCall.future( - _streaming_input_request_iterator(), + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), test_constants.LONG_TIMEOUT) response = response_future.result() expected_response = methods.StreamingInputCall( - _streaming_input_request_iterator(), 'not a real RpcContext!') + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), + 'not a real RpcContext!') self.assertEqual(expected_response, response) def testStreamingInputCallFutureExpired(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.pause(): response_future = stub.StreamingInputCall.future( - _streaming_input_request_iterator(), + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), test_constants.SHORT_TIMEOUT) with self.assertRaises(face.ExpirationError): response_future.result() @@ -363,10 +506,14 @@ class PythonPluginTest(unittest.TestCase): face.ExpirationError) def testStreamingInputCallFutureCancelled(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.pause(): response_future = stub.StreamingInputCall.future( - _streaming_input_request_iterator(), + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), test_constants.LONG_TIMEOUT) response_future.cancel() self.assertTrue(response_future.cancelled()) @@ -374,26 +521,38 @@ class PythonPluginTest(unittest.TestCase): response_future.result() def testStreamingInputCallFutureFailed(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.fail(): response_future = stub.StreamingInputCall.future( - _streaming_input_request_iterator(), + _streaming_input_request_iterator(self._payload_pb2, + self._requests_pb2), test_constants.LONG_TIMEOUT) self.assertIsNotNone(response_future.exception()) def testFullDuplexCall(self): - with _CreateService() as (methods, stub): - responses = stub.FullDuplexCall(_full_duplex_request_iterator(), - test_constants.LONG_TIMEOUT) + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + responses = stub.FullDuplexCall( + _full_duplex_request_iterator(self._requests_pb2), + test_constants.LONG_TIMEOUT) expected_responses = methods.FullDuplexCall( - _full_duplex_request_iterator(), 'not a real RpcContext!') + _full_duplex_request_iterator(self._requests_pb2), + 'not a real RpcContext!') for expected_response, response in moves.zip_longest( expected_responses, responses): self.assertEqual(expected_response, response) def testFullDuplexCallExpired(self): - request_iterator = _full_duplex_request_iterator() - with _CreateService() as (methods, stub): + self._protoc() + + request_iterator = _full_duplex_request_iterator(self._requests_pb2) + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.pause(): responses = stub.FullDuplexCall(request_iterator, test_constants.SHORT_TIMEOUT) @@ -401,8 +560,11 @@ class PythonPluginTest(unittest.TestCase): list(responses) def testFullDuplexCallCancelled(self): - with _CreateService() as (methods, stub): - request_iterator = _full_duplex_request_iterator() + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): + request_iterator = _full_duplex_request_iterator(self._requests_pb2) responses = stub.FullDuplexCall(request_iterator, test_constants.LONG_TIMEOUT) next(responses) @@ -411,8 +573,11 @@ class PythonPluginTest(unittest.TestCase): next(responses) def testFullDuplexCallFailed(self): - request_iterator = _full_duplex_request_iterator() - with _CreateService() as (methods, stub): + self._protoc() + + request_iterator = _full_duplex_request_iterator(self._requests_pb2) + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with methods.fail(): responses = stub.FullDuplexCall(request_iterator, test_constants.LONG_TIMEOUT) @@ -421,13 +586,16 @@ class PythonPluginTest(unittest.TestCase): next(responses) def testHalfDuplexCall(self): - with _CreateService() as (methods, stub): + self._protoc() + + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): def half_duplex_request_iterator(): - request = request_pb2.StreamingOutputCallRequest() + request = self._requests_pb2.StreamingOutputCallRequest() request.response_parameters.add(size=1, interval_us=0) yield request - request = request_pb2.StreamingOutputCallRequest() + request = self._requests_pb2.StreamingOutputCallRequest() request.response_parameters.add(size=2, interval_us=0) request.response_parameters.add(size=3, interval_us=0) yield request @@ -441,6 +609,8 @@ class PythonPluginTest(unittest.TestCase): self.assertEqual(expected_response, response) def testHalfDuplexCallWedged(self): + self._protoc() + condition = threading.Condition() wait_cell = [False] @@ -455,14 +625,15 @@ class PythonPluginTest(unittest.TestCase): condition.notify_all() def half_duplex_request_iterator(): - request = request_pb2.StreamingOutputCallRequest() + request = self._requests_pb2.StreamingOutputCallRequest() request.response_parameters.add(size=1, interval_us=0) yield request with condition: while wait_cell[0]: condition.wait() - with _CreateService() as (methods, stub): + with _CreateService(self._payload_pb2, self._responses_pb2, + self._service_pb2) as (methods, stub): with wait(): responses = stub.HalfDuplexCall(half_duplex_request_iterator(), test_constants.SHORT_TIMEOUT) From 31ad68a7e0d7f2bb6c3e6684782a88887b5190cc Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 18 Sep 2017 11:04:58 -0700 Subject: [PATCH 31/41] fix a bad typo --- src/core/lib/iomgr/closure.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c index bb9e437b08f..05401bc2bdc 100644 --- a/src/core/lib/iomgr/closure.c +++ b/src/core/lib/iomgr/closure.c @@ -171,9 +171,9 @@ void grpc_closure_sched(grpc_exec_ctx *exec_ctx, grpc_closure *c, gpr_log(GPR_ERROR, "Closure already scheduled. (closure: %p, created: [%s:%d], " "previously scheduled at: [%s: %d] run?: %s", - closure, closure->file_created, closure->line_created, - closure->file_initiated, closure->line_initiated, - closure->run ? "true" : "false"); + c, c->file_created, c->line_created, + c->file_initiated, c->line_initiated, + c->run ? "true" : "false"); abort(); } c->scheduled = true; @@ -203,9 +203,9 @@ void grpc_closure_list_sched(grpc_exec_ctx *exec_ctx, grpc_closure_list *list) { gpr_log(GPR_ERROR, "Closure already scheduled. (closure: %p, created: [%s:%d], " "previously scheduled at: [%s: %d] run?: %s", - closure, closure->file_created, closure->line_created, - closure->file_initiated, closure->line_initiated, - closure->run ? "true" : "false"); + c, c->file_created, c->line_created, + c->file_initiated, c->line_initiated, + c->run ? "true" : "false"); abort(); } c->scheduled = true; From 9667e8a57d0e4978f77a7fb35fab175dec2c9e10 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 18 Sep 2017 11:19:54 -0700 Subject: [PATCH 32/41] clang format --- src/core/lib/iomgr/closure.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c index 05401bc2bdc..00edefc6ae4 100644 --- a/src/core/lib/iomgr/closure.c +++ b/src/core/lib/iomgr/closure.c @@ -171,9 +171,8 @@ void grpc_closure_sched(grpc_exec_ctx *exec_ctx, grpc_closure *c, gpr_log(GPR_ERROR, "Closure already scheduled. (closure: %p, created: [%s:%d], " "previously scheduled at: [%s: %d] run?: %s", - c, c->file_created, c->line_created, - c->file_initiated, c->line_initiated, - c->run ? "true" : "false"); + c, c->file_created, c->line_created, c->file_initiated, + c->line_initiated, c->run ? "true" : "false"); abort(); } c->scheduled = true; @@ -203,9 +202,8 @@ void grpc_closure_list_sched(grpc_exec_ctx *exec_ctx, grpc_closure_list *list) { gpr_log(GPR_ERROR, "Closure already scheduled. (closure: %p, created: [%s:%d], " "previously scheduled at: [%s: %d] run?: %s", - c, c->file_created, c->line_created, - c->file_initiated, c->line_initiated, - c->run ? "true" : "false"); + c, c->file_created, c->line_created, c->file_initiated, + c->line_initiated, c->run ? "true" : "false"); abort(); } c->scheduled = true; From 4cd56837465234b7384bd3cb98abebced881f104 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Mon, 18 Sep 2017 11:22:03 -0700 Subject: [PATCH 33/41] Split C and C++ basic tests --- .../pull_request/grpc_basictests_c_dbg.cfg | 30 +++++++++++++++++++ .../pull_request/grpc_basictests_c_opt.cfg | 30 +++++++++++++++++++ .../pull_request/grpc_basictests_cpp_dbg.cfg | 30 +++++++++++++++++++ .../pull_request/grpc_basictests_cpp_opt.cfg | 30 +++++++++++++++++++ 4 files changed, 120 insertions(+) create mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_c_dbg.cfg create mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_c_opt.cfg create mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_cpp_dbg.cfg create mode 100644 tools/internal_ci/linux/pull_request/grpc_basictests_cpp_opt.cfg diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_c_dbg.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_c_dbg.cfg new file mode 100644 index 00000000000..577cb28ae5f --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_basictests_c_dbg.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" +timeout_mins: 240 +action { + define_artifacts { + regex: "**/*sponge_log.xml" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests linux c dbg --inner_jobs 16 -j 1 --internal_ci --max_time=3600" +} diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_c_opt.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_c_opt.cfg new file mode 100644 index 00000000000..9e0b724b2e1 --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_basictests_c_opt.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" +timeout_mins: 240 +action { + define_artifacts { + regex: "**/*sponge_log.xml" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests linux c opt --inner_jobs 16 -j 1 --internal_ci --max_time=3600" +} diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_dbg.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_dbg.cfg new file mode 100644 index 00000000000..0fda74cf44e --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_dbg.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" +timeout_mins: 240 +action { + define_artifacts { + regex: "**/*sponge_log.xml" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests linux c++ dbg --inner_jobs 16 -j 1 --internal_ci --max_time=3600" +} diff --git a/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_opt.cfg b/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_opt.cfg new file mode 100644 index 00000000000..199a8905d94 --- /dev/null +++ b/tools/internal_ci/linux/pull_request/grpc_basictests_cpp_opt.cfg @@ -0,0 +1,30 @@ +# 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. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_run_tests_matrix.sh" +timeout_mins: 240 +action { + define_artifacts { + regex: "**/*sponge_log.xml" + regex: "github/grpc/reports/**" + } +} + +env_vars { + key: "RUN_TESTS_FLAGS" + value: "-f basictests linux c++ opt --inner_jobs 16 -j 1 --internal_ci --max_time=3600" +} From 998bd2c7f46dc4bdd0e3381fcd1f4430fe3c3e32 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 18 Sep 2017 12:41:07 -0700 Subject: [PATCH 34/41] Fix leak in grpclb's lb_on_response_received_locked --- src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.c | 3 +++ 1 file changed, 3 insertions(+) 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 a776a07d99a..4fcfa745f1d 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 @@ -1560,6 +1560,9 @@ static void lb_on_response_received_locked(grpc_exec_ctx *exec_ctx, void *arg, exec_ctx, glb_policy->lb_call, ops, (size_t)(op - ops), &glb_policy->lb_on_response_received); /* loop */ GPR_ASSERT(GRPC_CALL_OK == call_error); + } else { + GRPC_LB_POLICY_WEAK_UNREF(exec_ctx, &glb_policy->base, + "lb_on_response_received_locked_shutdown"); } } else { /* empty payload: call cancelled. */ /* dispose of the "lb_on_response_received_locked" weak ref taken in From 6a8c0ac2aaecacbdf28a7414109e175297cec003 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Sun, 14 May 2017 23:18:05 -0700 Subject: [PATCH 35/41] Fix ping policy --- .../chttp2/transport/chttp2_transport.c | 4 ++-- .../ext/transport/chttp2/transport/parsing.c | 5 +++++ .../ext/transport/chttp2/transport/writing.c | 16 +++++++++------- test/core/end2end/tests/bad_ping.c | 2 +- test/core/end2end/tests/ping.c | 5 ++++- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 79a9ed827f7..3a28762224d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -557,8 +557,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, } } - t->ping_state.pings_before_data_required = - t->ping_policy.max_pings_without_data; + /* No pings allowed before receiving a header or data frame. */ + t->ping_state.pings_before_data_required = 0; t->ping_state.is_delayed_ping_timer_set = false; t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index 6c12c91365e..e52e19f3d2c 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -383,6 +383,8 @@ error_handler: /* t->parser = grpc_chttp2_data_parser_parse;*/ t->parser = grpc_chttp2_data_parser_parse; t->parser_data = &s->data_parser; + t->ping_state.pings_before_data_required = + t->ping_policy.max_pings_without_data; return GRPC_ERROR_NONE; } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, NULL)) { /* handle stream errors by closing the stream */ @@ -559,6 +561,9 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx, (t->incoming_frame_flags & GRPC_CHTTP2_DATA_FLAG_END_STREAM) != 0; } + t->ping_state.pings_before_data_required = + t->ping_policy.max_pings_without_data; + /* could be a new grpc_chttp2_stream or an existing grpc_chttp2_stream */ s = grpc_chttp2_parsing_lookup_stream(t, t->incoming_stream_id); if (s == NULL) { diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index 3ded801985d..d3699301e38 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -66,9 +66,17 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, } return; } + if (t->keepalive_permit_without_calls == 0 && + grpc_chttp2_stream_map_size(&t->stream_map) == 0) { + if (GRPC_TRACER_ON(grpc_http_trace) || + GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { + gpr_log(GPR_DEBUG, "Ping delayed [%p]: no active stream", t->peer_string); + } + return; + } if (t->ping_state.pings_before_data_required == 0 && t->ping_policy.max_pings_without_data != 0) { - /* need to send something of substance before sending a ping again */ + /* need to receive something of substance before sending a ping again */ if (GRPC_TRACER_ON(grpc_http_trace) || GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { gpr_log(GPR_DEBUG, "Ping delayed [%p]: too many recent pings: %d/%d", @@ -297,8 +305,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( grpc_slice_buffer_add( &t->outbuf, grpc_chttp2_window_update_create(s->id, stream_announce, &s->stats.outgoing)); - t->ping_state.pings_before_data_required = - t->ping_policy.max_pings_without_data; if (!t->is_client) { t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); @@ -375,8 +381,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( send_bytes); s->sending_bytes += send_bytes; } - t->ping_state.pings_before_data_required = - t->ping_policy.max_pings_without_data; if (!t->is_client) { t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); @@ -487,8 +491,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( grpc_slice_buffer_add( &t->outbuf, grpc_chttp2_window_update_create(0, transport_announce, &throwaway_stats)); - t->ping_state.pings_before_data_required = - t->ping_policy.max_pings_without_data; if (!t->is_client) { t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); diff --git a/test/core/end2end/tests/bad_ping.c b/test/core/end2end/tests/bad_ping.c index 12aceda6881..6713597fccf 100644 --- a/test/core/end2end/tests/bad_ping.c +++ b/test/core/end2end/tests/bad_ping.c @@ -71,7 +71,7 @@ static void test_bad_ping(grpc_end2end_test_config config) { .value.integer = 0}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, - .value.integer = 20}, + .value.integer = 0}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_BDP_PROBE, .value.integer = 0}}; diff --git a/test/core/end2end/tests/ping.c b/test/core/end2end/tests/ping.c index 112ad9d7d2a..b4d9b13a93d 100644 --- a/test/core/end2end/tests/ping.c +++ b/test/core/end2end/tests/ping.c @@ -42,7 +42,10 @@ static void test_ping(grpc_end2end_test_config config, .value.integer = 0}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, - .value.integer = 20}}; + .value.integer = 0}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, + .value.integer = 1}}; grpc_arg server_a[] = { {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS, From 27411495f6784a79890d676622e1c0d2f2a53584 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 15 May 2017 11:57:30 -0700 Subject: [PATCH 36/41] Fix keepalive_timeout --- test/core/end2end/tests/keepalive_timeout.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/core/end2end/tests/keepalive_timeout.c b/test/core/end2end/tests/keepalive_timeout.c index e0ead4ab62a..8d01f23c005 100644 --- a/test/core/end2end/tests/keepalive_timeout.c +++ b/test/core/end2end/tests/keepalive_timeout.c @@ -106,13 +106,13 @@ static void test_keepalive_timeout(grpc_end2end_test_config config) { .value.integer = 0}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_BDP_PROBE, - .value.integer = 1}}; + .value.integer = 0}}; - grpc_channel_args *client_args = NULL; - client_args = grpc_channel_args_copy_and_add(client_args, keepalive_args, 2); + grpc_channel_args client_args = {.num_args = GPR_ARRAY_SIZE(keepalive_args), + .args = keepalive_args}; grpc_end2end_test_fixture f = - begin_test(config, "keepalive_timeout", client_args, NULL); + begin_test(config, "keepalive_timeout", &client_args, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_op ops[6]; grpc_op *op; @@ -216,12 +216,6 @@ static void test_keepalive_timeout(grpc_end2end_test_config config) { grpc_byte_buffer_destroy(response_payload); grpc_byte_buffer_destroy(response_payload_recv); - if (client_args != NULL) { - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - grpc_channel_args_destroy(&exec_ctx, client_args); - grpc_exec_ctx_finish(&exec_ctx); - } - end_test(&f); config.tear_down_data(&f); } From ad32075af84eba51466ef53ae68c2e6e0d899ee6 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 15 May 2017 15:38:06 -0700 Subject: [PATCH 37/41] Set last_ping_sent_time to inf_past after receiving data --- include/grpc/impl/codegen/grpc_types.h | 13 ++-- .../chttp2/transport/chttp2_transport.c | 62 +++++++++++-------- .../transport/chttp2/transport/frame_ping.c | 2 +- .../ext/transport/chttp2/transport/internal.h | 4 +- .../ext/transport/chttp2/transport/parsing.c | 2 + .../ext/transport/chttp2/transport/writing.c | 41 ++++++------ test/core/client_channel/lb_policies_test.c | 2 +- test/core/end2end/tests/bad_ping.c | 21 ++++--- test/core/end2end/tests/ping.c | 21 ++++--- 9 files changed, 95 insertions(+), 73 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 748dc717a33..90f03f49a3e 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -188,9 +188,14 @@ typedef struct { #define GRPC_ARG_HTTP2_MAX_FRAME_SIZE "grpc.http2.max_frame_size" /** Should BDP probing be performed? */ #define GRPC_ARG_HTTP2_BDP_PROBE "grpc.http2.bdp_probe" -/** Minimum time (in milliseconds) between successive ping frames being sent */ -#define GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS \ +/** Minimum time between sending successive ping frames without receiving any + data frame, Int valued, milliseconds. */ +#define GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS \ "grpc.http2.min_time_between_pings_ms" +/** Minimum allowed time between receiving successive ping frames without + sending any data frame. Int valued, milliseconds */ +#define GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS \ + "grpc.http2.min_ping_interval_without_data_ms" /** Channel arg to override the http2 :scheme header */ #define GRPC_ARG_HTTP2_SCHEME "grpc.http2_scheme" /** How many pings can we send before needing to send a data frame or header @@ -202,10 +207,6 @@ typedef struct { closing the transport? (0 indicates that the server can bear an infinite number of misbehaving pings) */ #define GRPC_ARG_HTTP2_MAX_PING_STRIKES "grpc.http2.max_ping_strikes" -/** Minimum allowed time between two pings without sending any data frame. Int - valued, seconds */ -#define GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS \ - "grpc.http2.min_ping_interval_without_data_ms" /** How much data are we willing to queue up per stream if GRPC_WRITE_BUFFER_HINT is set? This is an upper bound */ #define GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE "grpc.http2.write_buffer_size" diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 3a28762224d..c742444902c 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -152,10 +152,10 @@ static void send_ping_locked( static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_error *error); -#define DEFAULT_MIN_TIME_BETWEEN_PINGS_MS 0 -#define DEFAULT_MAX_PINGS_BETWEEN_DATA 3 +#define DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ +#define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ +#define DEFAULT_MAX_PINGS_BETWEEN_DATA 2 #define DEFAULT_MAX_PING_STRIKES 2 -#define DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ /** keepalive-relevant functions */ static void init_keepalive_ping_locked(grpc_exec_ctx *exec_ctx, void *arg, @@ -364,11 +364,11 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->ping_policy = (grpc_chttp2_repeated_ping_policy){ .max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA, - .min_time_between_pings = - gpr_time_from_millis(DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, GPR_TIMESPAN), + .min_sent_ping_interval_without_data = gpr_time_from_millis( + DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN), .max_ping_strikes = DEFAULT_MAX_PING_STRIKES, - .min_ping_interval_without_data = gpr_time_from_millis( - DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN), + .min_recv_ping_interval_without_data = gpr_time_from_millis( + DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN), }; /* Keepalive setting */ @@ -434,23 +434,30 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->ping_policy.max_ping_strikes = grpc_channel_arg_get_integer( &channel_args->args[i], (grpc_integer_options){DEFAULT_MAX_PING_STRIKES, 0, INT_MAX}); - } else if (0 == strcmp(channel_args->args[i].key, - GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS)) { - t->ping_policy.min_time_between_pings = gpr_time_from_millis( - grpc_channel_arg_get_integer( - &channel_args->args[i], - (grpc_integer_options){DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, 0, - INT_MAX}), - GPR_TIMESPAN); } else if (0 == - strcmp(channel_args->args[i].key, - GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS)) { - t->ping_policy.min_ping_interval_without_data = gpr_time_from_millis( - grpc_channel_arg_get_integer( - &channel_args->args[i], - (grpc_integer_options){ - DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS, 0, INT_MAX}), - GPR_TIMESPAN); + strcmp( + channel_args->args[i].key, + GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS)) { + t->ping_policy.min_sent_ping_interval_without_data = + gpr_time_from_millis( + grpc_channel_arg_get_integer( + &channel_args->args[i], + (grpc_integer_options){ + DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, 0, + INT_MAX}), + GPR_TIMESPAN); + } else if (0 == + strcmp( + channel_args->args[i].key, + GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS)) { + t->ping_policy.min_recv_ping_interval_without_data = + gpr_time_from_millis( + grpc_channel_arg_get_integer( + &channel_args->args[i], + (grpc_integer_options){ + DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, 0, + INT_MAX}), + GPR_TIMESPAN); } else if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE)) { t->write_buffer_size = (uint32_t)grpc_channel_arg_get_integer( @@ -625,6 +632,9 @@ static void close_transport_locked(grpc_exec_ctx *exec_ctx, connectivity_state_set(exec_ctx, t, GRPC_CHANNEL_SHUTDOWN, GRPC_ERROR_REF(error), "close_transport"); grpc_endpoint_shutdown(exec_ctx, t->ep, GRPC_ERROR_REF(error)); + if (t->ping_state.is_delayed_ping_timer_set) { + grpc_timer_cancel(exec_ctx, &t->ping_state.delayed_ping_timer); + } switch (t->keepalive_state) { case GRPC_CHTTP2_KEEPALIVE_STATE_WAITING: grpc_timer_cancel(exec_ctx, &t->keepalive_ping_timer); @@ -1729,8 +1739,10 @@ static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_error *error) { grpc_chttp2_transport *t = (grpc_chttp2_transport *)tp; t->ping_state.is_delayed_ping_timer_set = false; - grpc_chttp2_initiate_write(exec_ctx, t, - GRPC_CHTTP2_INITIATE_WRITE_RETRY_SEND_PING); + if (error == GRPC_ERROR_NONE) { + grpc_chttp2_initiate_write(exec_ctx, t, + GRPC_CHTTP2_INITIATE_WRITE_RETRY_SEND_PING); + } } void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c index 81bd02ae702..d431d6b2dfa 100644 --- a/src/core/ext/transport/chttp2/transport/frame_ping.c +++ b/src/core/ext/transport/chttp2/transport/frame_ping.c @@ -92,7 +92,7 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser, gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec next_allowed_ping = gpr_time_add(t->ping_recv_state.last_ping_recv_time, - t->ping_policy.min_ping_interval_without_data); + t->ping_policy.min_recv_ping_interval_without_data); if (t->keepalive_permit_without_calls == 0 && grpc_chttp2_stream_map_size(&t->stream_map) == 0) { diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index c2dfce7c9c7..1682be28dd9 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -112,10 +112,10 @@ typedef struct { } grpc_chttp2_ping_queue; typedef struct { - gpr_timespec min_time_between_pings; int max_pings_without_data; int max_ping_strikes; - gpr_timespec min_ping_interval_without_data; + gpr_timespec min_sent_ping_interval_without_data; + gpr_timespec min_recv_ping_interval_without_data; } grpc_chttp2_repeated_ping_policy; typedef struct { diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c index e52e19f3d2c..3db1ad41231 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.c +++ b/src/core/ext/transport/chttp2/transport/parsing.c @@ -385,6 +385,7 @@ error_handler: t->parser_data = &s->data_parser; t->ping_state.pings_before_data_required = t->ping_policy.max_pings_without_data; + t->ping_state.last_ping_sent_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); return GRPC_ERROR_NONE; } else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, NULL)) { /* handle stream errors by closing the stream */ @@ -563,6 +564,7 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx, t->ping_state.pings_before_data_required = t->ping_policy.max_pings_without_data; + t->ping_state.last_ping_sent_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); /* could be a new grpc_chttp2_stream or an existing grpc_chttp2_stream */ s = grpc_chttp2_parsing_lookup_stream(t, t->incoming_stream_id); diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index d3699301e38..d61063ed31d 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -66,14 +66,6 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, } return; } - if (t->keepalive_permit_without_calls == 0 && - grpc_chttp2_stream_map_size(&t->stream_map) == 0) { - if (GRPC_TRACER_ON(grpc_http_trace) || - GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { - gpr_log(GPR_DEBUG, "Ping delayed [%p]: no active stream", t->peer_string); - } - return; - } if (t->ping_state.pings_before_data_required == 0 && t->ping_policy.max_pings_without_data != 0) { /* need to receive something of substance before sending a ping again */ @@ -86,11 +78,18 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, return; } gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); - gpr_timespec elapsed = gpr_time_sub(now, t->ping_state.last_ping_sent_time); - /*gpr_log(GPR_DEBUG, "elapsed:%d.%09d min:%d.%09d", (int)elapsed.tv_sec, - elapsed.tv_nsec, (int)t->ping_policy.min_time_between_pings.tv_sec, - (int)t->ping_policy.min_time_between_pings.tv_nsec);*/ - if (gpr_time_cmp(elapsed, t->ping_policy.min_time_between_pings) < 0) { + gpr_timespec next_allowed_ping = + gpr_time_add(t->ping_state.last_ping_sent_time, + t->ping_policy.min_sent_ping_interval_without_data); + if (t->keepalive_permit_without_calls == 0 && + grpc_chttp2_stream_map_size(&t->stream_map) == 0) { + next_allowed_ping = gpr_time_add(t->ping_recv_state.last_ping_recv_time, + gpr_time_from_seconds(7200, GPR_TIMESPAN)); + } + /*gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d", + (int)next_allowed_ping.tv_sec, (int)next_allowed_pingpsed.tv_nsec, + (int)now.tv_sec, (int)now.tv_nsec);*/ + if (gpr_time_cmp(next_allowed_ping, now) > 0) { /* not enough elapsed time between successive pings */ if (GRPC_TRACER_ON(grpc_http_trace) || GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { @@ -100,11 +99,11 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, } if (!t->ping_state.is_delayed_ping_timer_set) { t->ping_state.is_delayed_ping_timer_set = true; - grpc_timer_init(exec_ctx, &t->ping_state.delayed_ping_timer, - gpr_time_add(t->ping_state.last_ping_sent_time, - t->ping_policy.min_time_between_pings), - &t->retry_initiate_ping_locked, - gpr_now(GPR_CLOCK_MONOTONIC)); + grpc_timer_init( + exec_ctx, &t->ping_state.delayed_ping_timer, + gpr_time_add(t->ping_state.last_ping_sent_time, + t->ping_policy.min_sent_ping_interval_without_data), + &t->retry_initiate_ping_locked, gpr_now(GPR_CLOCK_MONOTONIC)); } return; } @@ -127,6 +126,12 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_ping_create(false, pq->inflight_id)); GRPC_STATS_INC_HTTP2_PINGS_SENT(exec_ctx); t->ping_state.last_ping_sent_time = now; + if (GRPC_TRACER_ON(grpc_http_trace) || + GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { + gpr_log(GPR_DEBUG, "Ping sent [%p]: %d/%d", t->peer_string, + t->ping_state.pings_before_data_required, + t->ping_policy.max_pings_without_data); + } t->ping_state.pings_before_data_required -= (t->ping_state.pings_before_data_required != 0); } diff --git a/test/core/client_channel/lb_policies_test.c b/test/core/client_channel/lb_policies_test.c index f70a9fc8808..ba37cd673f6 100644 --- a/test/core/client_channel/lb_policies_test.c +++ b/test/core/client_channel/lb_policies_test.c @@ -519,7 +519,7 @@ static grpc_channel *create_client(const servers_fixture *f) { arg_array[1].key = GRPC_ARG_LB_POLICY_NAME; arg_array[1].value.string = "ROUND_ROBIN"; arg_array[2].type = GRPC_ARG_INTEGER; - arg_array[2].key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS; + arg_array[2].key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS; arg_array[2].value.integer = 0; args.num_args = GPR_ARRAY_SIZE(arg_array); args.args = arg_array; diff --git a/test/core/end2end/tests/bad_ping.c b/test/core/end2end/tests/bad_ping.c index 6713597fccf..c97d11b3067 100644 --- a/test/core/end2end/tests/bad_ping.c +++ b/test/core/end2end/tests/bad_ping.c @@ -66,18 +66,19 @@ static void end_test(grpc_end2end_test_fixture *f) { static void test_bad_ping(grpc_end2end_test_config config) { grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); - grpc_arg client_a[] = {{.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS, - .value.integer = 0}, - {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, - .value.integer = 0}, - {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_BDP_PROBE, - .value.integer = 0}}; + grpc_arg client_a[] = { + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, + .value.integer = 10}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, + .value.integer = 0}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_HTTP2_BDP_PROBE, + .value.integer = 0}}; grpc_arg server_a[] = { {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS, + .key = GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, .value.integer = 300000 /* 5 minutes */}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MAX_PING_STRIKES, diff --git a/test/core/end2end/tests/ping.c b/test/core/end2end/tests/ping.c index b4d9b13a93d..23c82569ba8 100644 --- a/test/core/end2end/tests/ping.c +++ b/test/core/end2end/tests/ping.c @@ -37,18 +37,19 @@ static void test_ping(grpc_end2end_test_config config, grpc_connectivity_state state = GRPC_CHANNEL_IDLE; int i; - grpc_arg client_a[] = {{.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS, - .value.integer = 0}, - {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, - .value.integer = 0}, - {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, - .value.integer = 1}}; + grpc_arg client_a[] = { + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, + .value.integer = 10}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, + .value.integer = 0}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, + .value.integer = 1}}; grpc_arg server_a[] = { {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS, + .key = GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, .value.integer = 0}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS, From 1ecbebc08fe578236fbedefe770aa54b157221b4 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Thu, 18 May 2017 15:15:46 -0700 Subject: [PATCH 38/41] Set the ping behavior as described in the proposal --- .../transport/chttp2/transport/chttp2_transport.c | 2 +- src/core/ext/transport/chttp2/transport/writing.c | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index c742444902c..94dff495098 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -154,7 +154,7 @@ static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, #define DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ #define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ -#define DEFAULT_MAX_PINGS_BETWEEN_DATA 2 +#define DEFAULT_MAX_PINGS_BETWEEN_DATA 0 /* unlimited */ #define DEFAULT_MAX_PING_STRIKES 2 /** keepalive-relevant functions */ diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index d61063ed31d..8721803267d 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -86,9 +86,9 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, next_allowed_ping = gpr_time_add(t->ping_recv_state.last_ping_recv_time, gpr_time_from_seconds(7200, GPR_TIMESPAN)); } - /*gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d", - (int)next_allowed_ping.tv_sec, (int)next_allowed_pingpsed.tv_nsec, - (int)now.tv_sec, (int)now.tv_nsec);*/ + /* gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d", + (int)next_allowed_ping.tv_sec, (int)next_allowed_ping.tv_nsec, + (int)now.tv_sec, (int)now.tv_nsec); */ if (gpr_time_cmp(next_allowed_ping, now) > 0) { /* not enough elapsed time between successive pings */ if (GRPC_TRACER_ON(grpc_http_trace) || @@ -99,11 +99,9 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, } if (!t->ping_state.is_delayed_ping_timer_set) { t->ping_state.is_delayed_ping_timer_set = true; - grpc_timer_init( - exec_ctx, &t->ping_state.delayed_ping_timer, - gpr_time_add(t->ping_state.last_ping_sent_time, - t->ping_policy.min_sent_ping_interval_without_data), - &t->retry_initiate_ping_locked, gpr_now(GPR_CLOCK_MONOTONIC)); + grpc_timer_init(exec_ctx, &t->ping_state.delayed_ping_timer, + next_allowed_ping, &t->retry_initiate_ping_locked, + gpr_now(GPR_CLOCK_MONOTONIC)); } return; } From 18ea66fe3787ed27cc444a68451de05e34c4d024 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Thu, 22 Jun 2017 11:32:36 -0700 Subject: [PATCH 39/41] Update writing.c with the trailers-only response change --- src/core/ext/transport/chttp2/transport/writing.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index 8721803267d..7d9d4867e14 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -268,8 +268,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write( .stats = &s->stats.outgoing}; grpc_chttp2_encode_header(exec_ctx, &t->hpack_compressor, NULL, 0, s->send_initial_metadata, &hopt, &t->outbuf); - t->ping_state.pings_before_data_required = - t->ping_policy.max_pings_without_data; + now_writing = true; if (!t->is_client) { t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC); From d7eda6127af92a311f78c57dabc24f679d493ae3 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Thu, 29 Jun 2017 18:55:45 -0700 Subject: [PATCH 40/41] Add more options in grpc_chttp2_config_default_keepalive_args --- .../chttp2/transport/chttp2_transport.c | 64 +++++++++++++++---- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 94dff495098..144780b67b0 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -64,6 +64,11 @@ #define DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS false #define KEEPALIVE_TIME_BACKOFF_MULTIPLIER 2 +#define DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ +#define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ +#define DEFAULT_MAX_PINGS_BETWEEN_DATA 0 /* unlimited */ +#define DEFAULT_MAX_PING_STRIKES 2 + static int g_default_client_keepalive_time_ms = DEFAULT_CLIENT_KEEPALIVE_TIME_MS; static int g_default_client_keepalive_timeout_ms = @@ -75,6 +80,13 @@ static int g_default_server_keepalive_timeout_ms = static bool g_default_keepalive_permit_without_calls = DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS; +static int g_default_min_sent_ping_interval_without_data_ms = + DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS; +static int g_default_min_recv_ping_interval_without_data_ms = + DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS; +static int g_default_max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA; +static int g_default_max_ping_strikes = DEFAULT_MAX_PING_STRIKES; + #define MAX_CLIENT_STREAM_ID 0x7fffffffu grpc_tracer_flag grpc_http_trace = GRPC_TRACER_INITIALIZER(false, "http"); grpc_tracer_flag grpc_flowctl_trace = GRPC_TRACER_INITIALIZER(false, "flowctl"); @@ -152,11 +164,6 @@ static void send_ping_locked( static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_error *error); -#define DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ -#define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */ -#define DEFAULT_MAX_PINGS_BETWEEN_DATA 0 /* unlimited */ -#define DEFAULT_MAX_PING_STRIKES 2 - /** keepalive-relevant functions */ static void init_keepalive_ping_locked(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error); @@ -363,12 +370,12 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA, 1); t->ping_policy = (grpc_chttp2_repeated_ping_policy){ - .max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA, + .max_pings_without_data = g_default_max_pings_without_data, .min_sent_ping_interval_without_data = gpr_time_from_millis( - DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN), - .max_ping_strikes = DEFAULT_MAX_PING_STRIKES, + g_default_min_sent_ping_interval_without_data_ms, GPR_TIMESPAN), + .max_ping_strikes = g_default_max_ping_strikes, .min_recv_ping_interval_without_data = gpr_time_from_millis( - DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN), + g_default_min_recv_ping_interval_without_data_ms, GPR_TIMESPAN), }; /* Keepalive setting */ @@ -428,12 +435,13 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) { t->ping_policy.max_pings_without_data = grpc_channel_arg_get_integer( &channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_PINGS_BETWEEN_DATA, 0, INT_MAX}); + (grpc_integer_options){g_default_max_pings_without_data, 0, + INT_MAX}); } else if (0 == strcmp(channel_args->args[i].key, GRPC_ARG_HTTP2_MAX_PING_STRIKES)) { t->ping_policy.max_ping_strikes = grpc_channel_arg_get_integer( &channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_PING_STRIKES, 0, INT_MAX}); + (grpc_integer_options){g_default_max_ping_strikes, 0, INT_MAX}); } else if (0 == strcmp( channel_args->args[i].key, @@ -443,7 +451,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_channel_arg_get_integer( &channel_args->args[i], (grpc_integer_options){ - DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, 0, + g_default_min_sent_ping_interval_without_data_ms, 0, INT_MAX}), GPR_TIMESPAN); } else if (0 == @@ -455,7 +463,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_channel_arg_get_integer( &channel_args->args[i], (grpc_integer_options){ - DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, 0, + g_default_min_recv_ping_interval_without_data_ms, 0, INT_MAX}), GPR_TIMESPAN); } else if (0 == strcmp(channel_args->args[i].key, @@ -2643,6 +2651,36 @@ void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args, &args->args[i], (grpc_integer_options){g_default_keepalive_permit_without_calls, 0, 1}); + } else if (0 == + strcmp(args->args[i].key, GRPC_ARG_HTTP2_MAX_PING_STRIKES)) { + g_default_max_ping_strikes = grpc_channel_arg_get_integer( + &args->args[i], + (grpc_integer_options){g_default_max_ping_strikes, 0, INT_MAX}); + } else if (0 == strcmp(args->args[i].key, + GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) { + g_default_max_pings_without_data = grpc_channel_arg_get_integer( + &args->args[i], (grpc_integer_options){ + g_default_max_pings_without_data, 0, INT_MAX}); + } else if (0 == + strcmp( + args->args[i].key, + GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS)) { + g_default_min_sent_ping_interval_without_data_ms = + grpc_channel_arg_get_integer( + &args->args[i], + (grpc_integer_options){ + g_default_min_sent_ping_interval_without_data_ms, 0, + INT_MAX}); + } else if (0 == + strcmp( + args->args[i].key, + GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS)) { + g_default_min_recv_ping_interval_without_data_ms = + grpc_channel_arg_get_integer( + &args->args[i], + (grpc_integer_options){ + g_default_min_recv_ping_interval_without_data_ms, 0, + INT_MAX}); } } } From a594ba0bd7be974cff1a8d1eeb3b21eb049e507e Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Thu, 7 Sep 2017 19:04:59 -0700 Subject: [PATCH 41/41] add a client side utility for extracting a google rpc status from a grpc::Status --- grpc.gemspec | 1 + src/ruby/lib/grpc/google_rpc_status_utils.rb | 28 +++ src/ruby/spec/google_rpc_status_utils_spec.rb | 223 ++++++++++++++++++ templates/grpc.gemspec.template | 1 + 4 files changed, 253 insertions(+) create mode 100644 src/ruby/lib/grpc/google_rpc_status_utils.rb create mode 100644 src/ruby/spec/google_rpc_status_utils_spec.rb diff --git a/grpc.gemspec b/grpc.gemspec index 2d0f9fd4504..96323aed106 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -29,6 +29,7 @@ Gem::Specification.new do |s| s.add_dependency 'google-protobuf', '~> 3.1' s.add_dependency 'googleauth', '~> 0.5.1' + s.add_dependency 'googleapis-common-protos-types', '~> 1.0.0' s.add_development_dependency 'bundler', '~> 1.9' s.add_development_dependency 'facter', '~> 2.4' diff --git a/src/ruby/lib/grpc/google_rpc_status_utils.rb b/src/ruby/lib/grpc/google_rpc_status_utils.rb new file mode 100644 index 00000000000..fdadd6b76e4 --- /dev/null +++ b/src/ruby/lib/grpc/google_rpc_status_utils.rb @@ -0,0 +1,28 @@ +# 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. + +require_relative './grpc' +require 'google/rpc/status_pb' + +# GRPC contains the General RPC module. +module GRPC + # GoogleRpcStatusUtils provides utilities to convert between a + # GRPC::Core::Status and a deserialized Google::Rpc::Status proto + class GoogleRpcStatusUtils + def self.extract_google_rpc_status(status) + fail ArgumentError, 'bad type' unless status.is_a? Struct::Status + Google::Rpc::Status.decode(status.metadata['grpc-status-details-bin']) + end + end +end diff --git a/src/ruby/spec/google_rpc_status_utils_spec.rb b/src/ruby/spec/google_rpc_status_utils_spec.rb new file mode 100644 index 00000000000..fe221c30ddd --- /dev/null +++ b/src/ruby/spec/google_rpc_status_utils_spec.rb @@ -0,0 +1,223 @@ +# 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. + +require 'grpc' +require_relative '../lib/grpc/google_rpc_status_utils' +require_relative '../pb/src/proto/grpc/testing/messages_pb' +require_relative '../pb/src/proto/grpc/testing/messages_pb' +require 'google/protobuf/well_known_types' + +include GRPC::Core + +describe 'conversion from a status struct to a google protobuf status' do + it 'fails if the input is not a status struct' do + begin + GRPC::GoogleRpcStatusUtils.extract_google_rpc_status('string') + rescue => e + exception = e + end + expect(exception.is_a?(ArgumentError)).to be true + expect(exception.message.include?('bad type')).to be true + end + + it 'fails with some error if the header key is missing' do + status = Struct::Status.new(1, 'details', key: 'val') + expect(status.metadata.nil?).to be false + expect do + GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + end.to raise_error(StandardError) + end + + it 'fails with some error if the header key fails to deserialize' do + status = Struct::Status.new(1, 'details', + 'grpc-status-details-bin' => 'string_val') + expect do + GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + end.to raise_error(StandardError) + end + + it 'silently ignores erroneous mismatch between messages in '\ + 'status struct and protobuf status' do + proto = Google::Rpc::Status.new(code: 1, message: 'proto message') + encoded_proto = Google::Rpc::Status.encode(proto) + status = Struct::Status.new(1, 'struct message', + 'grpc-status-details-bin' => encoded_proto) + rpc_status = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + expect(rpc_status).to eq(proto) + end + + it 'silently ignores erroneous mismatch between codes in status struct '\ + 'and protobuf status' do + proto = Google::Rpc::Status.new(code: 1, message: 'matching message') + encoded_proto = Google::Rpc::Status.encode(proto) + status = Struct::Status.new(2, 'matching message', + 'grpc-status-details-bin' => encoded_proto) + rpc_status = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + expect(rpc_status).to eq(proto) + end + + it 'can succesfully convert a status struct into a google protobuf status '\ + 'when there are no rpcstatus details' do + proto = Google::Rpc::Status.new(code: 1, message: 'matching message') + encoded_proto = Google::Rpc::Status.encode(proto) + status = Struct::Status.new(1, 'matching message', + 'grpc-status-details-bin' => encoded_proto) + out = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + expect(out.code).to eq(1) + expect(out.message).to eq('matching message') + expect(out.details).to eq([]) + end + + it 'can succesfully convert a status struct into a google protobuf '\ + 'status when there are multiple rpcstatus details' do + simple_request_any = Google::Protobuf::Any.new + simple_request = Grpc::Testing::SimpleRequest.new( + payload: Grpc::Testing::Payload.new(body: 'request')) + simple_request_any.pack(simple_request) + simple_response_any = Google::Protobuf::Any.new + simple_response = Grpc::Testing::SimpleResponse.new( + payload: Grpc::Testing::Payload.new(body: 'response')) + simple_response_any.pack(simple_response) + payload_any = Google::Protobuf::Any.new + payload = Grpc::Testing::Payload.new(body: 'payload') + payload_any.pack(payload) + proto = Google::Rpc::Status.new(code: 1, + message: 'matching message', + details: [ + simple_request_any, + simple_response_any, + payload_any + ]) + encoded_proto = Google::Rpc::Status.encode(proto) + status = Struct::Status.new(1, 'matching message', + 'grpc-status-details-bin' => encoded_proto) + out = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status(status) + expect(out.code).to eq(1) + expect(out.message).to eq('matching message') + expect(out.details[0].unpack( + Grpc::Testing::SimpleRequest)).to eq(simple_request) + expect(out.details[1].unpack( + Grpc::Testing::SimpleResponse)).to eq(simple_response) + expect(out.details[2].unpack( + Grpc::Testing::Payload)).to eq(payload) + end +end + +# Test message +class EchoMsg + def self.marshal(_o) + '' + end + + def self.unmarshal(_o) + EchoMsg.new + end +end + +# A test service that fills in the "reserved" grpc-status-details-bin trailer, +# for client-side testing of GoogleRpcStatus protobuf extraction from trailers. +class GoogleRpcStatusTestService + include GRPC::GenericService + rpc :an_rpc, EchoMsg, EchoMsg + + def initialize(encoded_rpc_status) + @encoded_rpc_status = encoded_rpc_status + end + + def an_rpc(_, _) + # TODO: create a server-side utility API for sending a google rpc status. + # Applications are not expected to set the grpc-status-details-bin + # ("grpc"-fixed and reserved for library use) manually. + # Doing so here is only for testing of the client-side api for extracting + # a google rpc status, which is useful + # when the interacting with a server that does fill in this trailer. + fail GRPC::Unknown.new('test message', + 'grpc-status-details-bin' => @encoded_rpc_status) + end +end + +GoogleRpcStatusTestStub = GoogleRpcStatusTestService.rpc_stub_class + +describe 'receving a google rpc status from a remote endpoint' do + def start_server(encoded_rpc_status) + @srv = GRPC::RpcServer.new(pool_size: 1) + @server_port = @srv.add_http2_port('localhost:0', + :this_port_is_insecure) + @srv.handle(GoogleRpcStatusTestService.new(encoded_rpc_status)) + @server_thd = Thread.new { @srv.run } + @srv.wait_till_running + end + + def stop_server + expect(@srv.stopped?).to be(false) + @srv.stop + @server_thd.join + expect(@srv.stopped?).to be(true) + end + + before(:each) do + simple_request_any = Google::Protobuf::Any.new + simple_request = Grpc::Testing::SimpleRequest.new( + payload: Grpc::Testing::Payload.new(body: 'request')) + simple_request_any.pack(simple_request) + simple_response_any = Google::Protobuf::Any.new + simple_response = Grpc::Testing::SimpleResponse.new( + payload: Grpc::Testing::Payload.new(body: 'response')) + simple_response_any.pack(simple_response) + payload_any = Google::Protobuf::Any.new + payload = Grpc::Testing::Payload.new(body: 'payload') + payload_any.pack(payload) + @expected_proto = Google::Rpc::Status.new( + code: StatusCodes::UNKNOWN, + message: 'test message', + details: [simple_request_any, simple_response_any, payload_any]) + start_server(Google::Rpc::Status.encode(@expected_proto)) + end + + after(:each) do + stop_server + end + + it 'should receive be able to extract a google rpc status from the '\ + 'status struct taken from a BadStatus exception' do + stub = GoogleRpcStatusTestStub.new("localhost:#{@server_port}", + :this_channel_is_insecure) + begin + stub.an_rpc(EchoMsg.new) + rescue GRPC::BadStatus => e + rpc_status = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status( + e.to_status) + end + expect(rpc_status).to eq(@expected_proto) + end + + it 'should receive be able to extract a google rpc status from the '\ + 'status struct taken from the op view of a call' do + stub = GoogleRpcStatusTestStub.new("localhost:#{@server_port}", + :this_channel_is_insecure) + op = stub.an_rpc(EchoMsg.new, return_op: true) + begin + op.execute + rescue GRPC::BadStatus => e + status_from_exception = e.to_status + end + rpc_status = GRPC::GoogleRpcStatusUtils.extract_google_rpc_status( + op.status) + expect(rpc_status).to eq(@expected_proto) + # "to_status" on the bad status should give the same result + # as "status" on the "op view". + expect(GRPC::GoogleRpcStatusUtils.extract_google_rpc_status( + status_from_exception)).to eq(rpc_status) + end +end diff --git a/templates/grpc.gemspec.template b/templates/grpc.gemspec.template index e62e5b27215..215d5f9df9a 100644 --- a/templates/grpc.gemspec.template +++ b/templates/grpc.gemspec.template @@ -31,6 +31,7 @@ s.add_dependency 'google-protobuf', '~> 3.1' s.add_dependency 'googleauth', '~> 0.5.1' + s.add_dependency 'googleapis-common-protos-types', '~> 1.0.0' s.add_development_dependency 'bundler', '~> 1.9' s.add_development_dependency 'facter', '~> 2.4'