From 072ebaa1537673e100b0a027d3935252da14fccb Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 1 Mar 2016 18:33:12 -0800 Subject: [PATCH 01/17] make python test suites run in parallel --- src/python/grpcio/tests/_runner.py | 11 +++- src/python/grpcio/tests/tests.json | 62 +++++++++++++++++++ .../grpcio/tests/unit/_sanity/__init__.py | 30 +++++++++ .../grpcio/tests/unit/_sanity/_sanity_test.py | 53 ++++++++++++++++ tools/run_tests/build_python.sh | 2 + tools/run_tests/run_python.sh | 7 ++- tools/run_tests/run_tests.py | 26 +++++--- 7 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 src/python/grpcio/tests/tests.json create mode 100644 src/python/grpcio/tests/unit/_sanity/__init__.py create mode 100644 src/python/grpcio/tests/unit/_sanity/_sanity_test.py diff --git a/src/python/grpcio/tests/_runner.py b/src/python/grpcio/tests/_runner.py index 4f1ddb57fcf..38a5432e791 100644 --- a/src/python/grpcio/tests/_runner.py +++ b/src/python/grpcio/tests/_runner.py @@ -1,4 +1,4 @@ -# Copyright 2015, Google Inc. +# Copyright 2015-2016, Google Inc. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -143,10 +143,17 @@ class Runner(object): def run(self, suite): """See setuptools' test_runner setup argument for information.""" + # only run test cases with id starting with given prefix + testcase_filter = os.getenv('GPRC_PYTHON_TESTRUNNER_FILTER') + filtered_cases = [] + for case in _loader.iterate_suite_cases(suite): + if not testcase_filter or case.id().startswith(testcase_filter): + filtered_cases.append(case) + # Ensure that every test case has no collision with any other test case in # the augmented results. augmented_cases = [AugmentedCase(case, uuid.uuid4()) - for case in _loader.iterate_suite_cases(suite)] + for case in filtered_cases] case_id_by_case = dict((augmented_case.case, augmented_case.id) for augmented_case in augmented_cases) result_out = StringIO.StringIO() diff --git a/src/python/grpcio/tests/tests.json b/src/python/grpcio/tests/tests.json new file mode 100644 index 00000000000..388d040d5ca --- /dev/null +++ b/src/python/grpcio/tests/tests.json @@ -0,0 +1,62 @@ +[ + "_base_interface_test.AsyncEasyTest", + "_base_interface_test.AsyncPeasyTest", + "_base_interface_test.SyncEasyTest", + "_base_interface_test.SyncPeasyTest", + "_beta_features_test.BetaFeaturesTest", + "_beta_features_test.ContextManagementAndLifecycleTest", + "_channel_test.ChannelTest", + "_connectivity_channel_test.ChannelConnectivityTest", + "_core_over_links_base_interface_test.AsyncEasyTest", + "_core_over_links_base_interface_test.AsyncPeasyTest", + "_core_over_links_base_interface_test.SyncEasyTest", + "_core_over_links_base_interface_test.SyncPeasyTest", + "_crust_over_core_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", + "_crust_over_core_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", + "_crust_over_core_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_over_links_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_over_links_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", + "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", + "_crust_over_core_over_links_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", + "_face_interface_test.DynamicInvokerBlockingInvocationInlineServiceTest", + "_face_interface_test.DynamicInvokerEventInvocationSynchronousEventServiceTest", + "_face_interface_test.DynamicInvokerFutureInvocationAsynchronousEventServiceTest", + "_face_interface_test.GenericInvokerBlockingInvocationInlineServiceTest", + "_face_interface_test.GenericInvokerEventInvocationSynchronousEventServiceTest", + "_face_interface_test.GenericInvokerFutureInvocationAsynchronousEventServiceTest", + "_face_interface_test.MultiCallableInvokerBlockingInvocationInlineServiceTest", + "_face_interface_test.MultiCallableInvokerEventInvocationSynchronousEventServiceTest", + "_face_interface_test.MultiCallableInvokerFutureInvocationAsynchronousEventServiceTest", + "_implementations_test.ChannelCredentialsTest", + "_insecure_interop_test.InsecureInteropTest", + "_intermediary_low_test.CancellationTest", + "_intermediary_low_test.EchoTest", + "_intermediary_low_test.ExpirationTest", + "_intermediary_low_test.LonelyClientTest", + "_later_test.LaterTest", + "_logging_pool_test.LoggingPoolTest", + "_lonely_invocation_link_test.LonelyInvocationLinkTest", + "_low_test.HangingServerShutdown", + "_low_test.InsecureServerInsecureClient", + "_not_found_test.NotFoundTest", + "_sanity_test.Sanity", + "_secure_interop_test.SecureInteropTest", + "_transmission_test.RoundTripTest", + "_transmission_test.TransmissionTest", + "_utilities_test.ChannelConnectivityTest", + "beta_python_plugin_test.PythonPluginTest", + "cygrpc_test.InsecureServerInsecureClient", + "cygrpc_test.SecureServerSecureClient", + "cygrpc_test.TypeSmokeTest" +] \ No newline at end of file diff --git a/src/python/grpcio/tests/unit/_sanity/__init__.py b/src/python/grpcio/tests/unit/_sanity/__init__.py new file mode 100644 index 00000000000..2f88fa04122 --- /dev/null +++ b/src/python/grpcio/tests/unit/_sanity/__init__.py @@ -0,0 +1,30 @@ +# Copyright 2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + + diff --git a/src/python/grpcio/tests/unit/_sanity/_sanity_test.py b/src/python/grpcio/tests/unit/_sanity/_sanity_test.py new file mode 100644 index 00000000000..0a5a715c0e1 --- /dev/null +++ b/src/python/grpcio/tests/unit/_sanity/_sanity_test.py @@ -0,0 +1,53 @@ +# Copyright 2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import json +import unittest + +import tests + + +class Sanity(unittest.TestCase): + + def testTestsJsonUpToDate(self): + """Autodiscovers all test suites and checks that tests.json is up to date""" + loader = tests.Loader() + loader.loadTestsFromNames(['tests']) + test_suite_names = [ + test_case_class.id().rsplit('.', 1)[0] + for test_case_class in tests._loader.iterate_suite_cases(loader.suite)] + test_suite_names = sorted(set(test_suite_names)) + + with open('src/python/grpcio/tests/tests.json') as tests_json_file: + tests_json = json.load(tests_json_file) + self.assertListEqual(test_suite_names, tests_json) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/tools/run_tests/build_python.sh b/tools/run_tests/build_python.sh index e0fcbb602d4..365abb4d863 100755 --- a/tools/run_tests/build_python.sh +++ b/tools/run_tests/build_python.sh @@ -45,3 +45,5 @@ export GRPC_PYTHON_ENABLE_CYTHON_TRACING=1 tox --notest $ROOT/.tox/py27/bin/python $ROOT/setup.py build +$ROOT/.tox/py27/bin/python $ROOT/setup.py build_py +$ROOT/.tox/py27/bin/python $ROOT/setup.py gather --test diff --git a/tools/run_tests/run_python.sh b/tools/run_tests/run_python.sh index ffe9c12af1a..beb747a6169 100755 --- a/tools/run_tests/run_python.sh +++ b/tools/run_tests/run_python.sh @@ -42,7 +42,12 @@ export LDFLAGS="-L$ROOT/libs/$CONFIG" export GRPC_PYTHON_BUILD_WITH_CYTHON=1 export GRPC_PYTHON_ENABLE_CYTHON_TRACING=1 -tox +if [ "$CONFIG" = "gcov" ] +then + tox +else + $ROOT/.tox/py27/bin/python $ROOT/setup.py test +fi mkdir -p $ROOT/reports rm -rf $ROOT/reports/python-coverage diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 08a5ff0e8fa..c55d1fbe633 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -353,15 +353,27 @@ class PythonLanguage(object): _check_compiler(self.args.compiler, ['default']) def test_specs(self): + # load list of known test suites + with open('src/python/grpcio/tests/tests.json') as tests_json_file: + tests_json = json.load(tests_json_file) environment = dict(_FORCE_ENVIRON_FOR_WRAPPERS) environment['PYVER'] = '2.7' - return [self.config.job_spec( - ['tools/run_tests/run_python.sh'], - None, - environ=environment, - shortname='py.test', - timeout_seconds=15*60 - )] + if self.config.build_config != 'gcov': + return [self.config.job_spec( + ['tools/run_tests/run_python.sh'], + None, + environ=dict(environment.items() + + [('GPRC_PYTHON_TESTRUNNER_FILTER', suite_name)]), + shortname='py.test.%s' % suite_name, + timeout_seconds=5*60) + for suite_name in tests_json] + else: + return [self.config.job_spec(['tools/run_tests/run_python.sh'], + None, + environ=environment, + shortname='py.test.coverage', + timeout_seconds=15*60)] + def pre_build_steps(self): return [] From 98990726a02829c29674f143ab6e02df5415514d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 2 Mar 2016 22:04:00 -0800 Subject: [PATCH 02/17] Revert "Update reconnect_interop_server.cc" --- test/cpp/interop/reconnect_interop_server.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/cpp/interop/reconnect_interop_server.cc b/test/cpp/interop/reconnect_interop_server.cc index 785f9c7ad5f..3602b8c2b05 100644 --- a/test/cpp/interop/reconnect_interop_server.cc +++ b/test/cpp/interop/reconnect_interop_server.cc @@ -30,8 +30,6 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */ - -// Test description at doc/connection-backoff-interop-test-description.md #include #include From 0cb803d9ca4286601e9e6a3240cfa3488b662b7c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 2 Mar 2016 22:17:24 -0800 Subject: [PATCH 03/17] Always ref writable streams We suffered a bug whereby doing a follow-up write to another write could resurrect a deleted stream, causing all sorts of crash. Fix: when a stream becomes writable (vs when we start writing) take a ref on the stream, and only relinquish it once we're done writing. --- grpc.def | 1 + include/grpc/impl/codegen/sync.h | 4 ++ src/core/iomgr/iomgr.c | 16 +++++++ src/core/iomgr/iomgr_internal.h | 6 ++- src/core/support/sync.c | 7 ++- src/core/transport/chttp2/internal.h | 18 +++++--- src/core/transport/chttp2/parsing.c | 6 +-- src/core/transport/chttp2/stream_lists.c | 38 ++++++++-------- src/core/transport/chttp2/writing.c | 37 ++++++---------- src/core/transport/chttp2_transport.c | 43 +++++++++++-------- src/core/transport/metadata.c | 8 ++++ src/core/transport/transport.c | 2 +- .../grpcio/grpc/_cython/imports.generated.c | 2 + .../grpcio/grpc/_cython/imports.generated.h | 3 ++ src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 + src/ruby/ext/grpc/rb_grpc_imports.generated.h | 3 ++ test/cpp/interop/reconnect_interop_client.cc | 2 +- 17 files changed, 127 insertions(+), 71 deletions(-) diff --git a/grpc.def b/grpc.def index bd0bc85a7c3..f81aa1b05a6 100644 --- a/grpc.def +++ b/grpc.def @@ -182,6 +182,7 @@ EXPORTS gpr_event_wait gpr_ref_init gpr_ref + gpr_ref_non_zero gpr_refn gpr_unref gpr_stats_init diff --git a/include/grpc/impl/codegen/sync.h b/include/grpc/impl/codegen/sync.h index d2f19d37d66..6fd7d64b299 100644 --- a/include/grpc/impl/codegen/sync.h +++ b/include/grpc/impl/codegen/sync.h @@ -182,6 +182,10 @@ GPRAPI void gpr_ref_init(gpr_refcount *r, int n); /* Increment the reference count *r. Requires *r initialized. */ GPRAPI void gpr_ref(gpr_refcount *r); +/* Increment the reference count *r. Requires *r initialized. + Crashes if refcount is zero */ +GPRAPI void gpr_ref_non_zero(gpr_refcount *r); + /* Increment the reference count *r by n. Requires *r initialized, n > 0. */ GPRAPI void gpr_refn(gpr_refcount *r, int n); diff --git a/src/core/iomgr/iomgr.c b/src/core/iomgr/iomgr.c index 04580150f3a..9c89c2c08a4 100644 --- a/src/core/iomgr/iomgr.c +++ b/src/core/iomgr/iomgr.c @@ -41,9 +41,11 @@ #include #include #include +#include #include "src/core/iomgr/iomgr_internal.h" #include "src/core/iomgr/timer.h" +#include "src/core/support/env.h" #include "src/core/support/string.h" static gpr_mu g_mu; @@ -116,6 +118,9 @@ void grpc_iomgr_shutdown(void) { "memory leaks are likely", count_objects()); dump_objects("LEAKED"); + if (grpc_iomgr_abort_on_leaks()) { + abort(); + } } break; } @@ -154,3 +159,14 @@ void grpc_iomgr_unregister_object(grpc_iomgr_object *obj) { gpr_mu_unlock(&g_mu); gpr_free(obj->name); } + +bool grpc_iomgr_abort_on_leaks(void) { + char *env = gpr_getenv("GRPC_ABORT_ON_LEAKS"); + if (env == NULL) return false; + static const char *truthy[] = {"yes", "Yes", "YES", "true", + "True", "TRUE", "1"}; + for (size_t i = 0; i < GPR_ARRAY_SIZE(truthy); i++) { + if (0 == strcmp(env, truthy[i])) return true; + } + return false; +} diff --git a/src/core/iomgr/iomgr_internal.h b/src/core/iomgr/iomgr_internal.h index e372c18e8a0..ac2c46ebe62 100644 --- a/src/core/iomgr/iomgr_internal.h +++ b/src/core/iomgr/iomgr_internal.h @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -34,6 +34,8 @@ #ifndef GRPC_INTERNAL_CORE_IOMGR_IOMGR_INTERNAL_H #define GRPC_INTERNAL_CORE_IOMGR_IOMGR_INTERNAL_H +#include + #include "src/core/iomgr/iomgr.h" #include @@ -55,4 +57,6 @@ void grpc_iomgr_platform_flush(void); /** tear down all platform specific global iomgr structures */ void grpc_iomgr_platform_shutdown(void); +bool grpc_iomgr_abort_on_leaks(void); + #endif /* GRPC_INTERNAL_CORE_IOMGR_IOMGR_INTERNAL_H */ diff --git a/src/core/support/sync.c b/src/core/support/sync.c index d368422d9ea..69e3e39c5ce 100644 --- a/src/core/support/sync.c +++ b/src/core/support/sync.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -98,6 +98,11 @@ void gpr_ref_init(gpr_refcount *r, int n) { gpr_atm_rel_store(&r->count, n); } void gpr_ref(gpr_refcount *r) { gpr_atm_no_barrier_fetch_add(&r->count, 1); } +void gpr_ref_non_zero(gpr_refcount *r) { + gpr_atm prior = gpr_atm_no_barrier_fetch_add(&r->count, 1); + GPR_ASSERT(prior > 0); +} + void gpr_refn(gpr_refcount *r, int n) { gpr_atm_no_barrier_fetch_add(&r->count, n); } diff --git a/src/core/transport/chttp2/internal.h b/src/core/transport/chttp2/internal.h index d76d31be23f..891aad6ef28 100644 --- a/src/core/transport/chttp2/internal.h +++ b/src/core/transport/chttp2/internal.h @@ -417,7 +417,7 @@ typedef struct { /** HTTP2 stream id for this stream, or zero if one has not been assigned */ uint32_t id; uint8_t fetching; - uint8_t sent_initial_metadata; + bool sent_initial_metadata; uint8_t sent_message; uint8_t sent_trailing_metadata; uint8_t read_closed; @@ -509,7 +509,7 @@ void grpc_chttp2_publish_reads(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_global *global, grpc_chttp2_transport_parsing *parsing); -void grpc_chttp2_list_add_writable_stream( +bool grpc_chttp2_list_add_writable_stream( grpc_chttp2_transport_global *transport_global, grpc_chttp2_stream_global *stream_global); /** Get a writable stream @@ -519,14 +519,13 @@ int grpc_chttp2_list_pop_writable_stream( grpc_chttp2_transport_writing *transport_writing, grpc_chttp2_stream_global **stream_global, grpc_chttp2_stream_writing **stream_writing); -void grpc_chttp2_list_remove_writable_stream( +bool grpc_chttp2_list_remove_writable_stream( grpc_chttp2_transport_global *transport_global, - grpc_chttp2_stream_global *stream_global); + grpc_chttp2_stream_global *stream_global) GRPC_MUST_USE_RESULT; -/* returns 1 if stream added, 0 if it was already present */ -int grpc_chttp2_list_add_writing_stream( +void grpc_chttp2_list_add_writing_stream( grpc_chttp2_transport_writing *transport_writing, - grpc_chttp2_stream_writing *stream_writing) GRPC_MUST_USE_RESULT; + grpc_chttp2_stream_writing *stream_writing); int grpc_chttp2_list_have_writing_streams( grpc_chttp2_transport_writing *transport_writing); int grpc_chttp2_list_pop_writing_stream( @@ -770,4 +769,9 @@ void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport_parsing *parsing, const uint8_t *opaque_8bytes); +/** add a ref to the stream and add it to the writable list; + ref will be dropped in writing.c */ +void grpc_chttp2_become_writable(grpc_chttp2_transport_global *transport_global, + grpc_chttp2_stream_global *stream_global); + #endif diff --git a/src/core/transport/chttp2/parsing.c b/src/core/transport/chttp2/parsing.c index 8fdebd7f139..0516f39fa9e 100644 --- a/src/core/transport/chttp2/parsing.c +++ b/src/core/transport/chttp2/parsing.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -149,7 +149,7 @@ void grpc_chttp2_publish_reads( if (was_zero && !is_zero) { while (grpc_chttp2_list_pop_stalled_by_transport(transport_global, &stream_global)) { - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } @@ -178,7 +178,7 @@ void grpc_chttp2_publish_reads( outgoing_window); is_zero = stream_global->outgoing_window <= 0; if (was_zero && !is_zero) { - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } stream_global->max_recv_bytes -= (uint32_t)GPR_MIN( diff --git a/src/core/transport/chttp2/stream_lists.c b/src/core/transport/chttp2/stream_lists.c index b284c788183..60fe735cfce 100644 --- a/src/core/transport/chttp2/stream_lists.c +++ b/src/core/transport/chttp2/stream_lists.c @@ -100,11 +100,14 @@ static void stream_list_remove(grpc_chttp2_transport *t, grpc_chttp2_stream *s, } } -static void stream_list_maybe_remove(grpc_chttp2_transport *t, +static bool stream_list_maybe_remove(grpc_chttp2_transport *t, grpc_chttp2_stream *s, grpc_chttp2_stream_list_id id) { if (s->included[id]) { stream_list_remove(t, s, id); + return true; + } else { + return false; } } @@ -125,23 +128,24 @@ static void stream_list_add_tail(grpc_chttp2_transport *t, s->included[id] = 1; } -static int stream_list_add(grpc_chttp2_transport *t, grpc_chttp2_stream *s, - grpc_chttp2_stream_list_id id) { +static bool stream_list_add(grpc_chttp2_transport *t, grpc_chttp2_stream *s, + grpc_chttp2_stream_list_id id) { if (s->included[id]) { - return 0; + return false; } stream_list_add_tail(t, s, id); - return 1; + return true; } /* wrappers for specializations */ -void grpc_chttp2_list_add_writable_stream( +bool grpc_chttp2_list_add_writable_stream( grpc_chttp2_transport_global *transport_global, grpc_chttp2_stream_global *stream_global) { GPR_ASSERT(stream_global->id != 0); - stream_list_add(TRANSPORT_FROM_GLOBAL(transport_global), - STREAM_FROM_GLOBAL(stream_global), GRPC_CHTTP2_LIST_WRITABLE); + return stream_list_add(TRANSPORT_FROM_GLOBAL(transport_global), + STREAM_FROM_GLOBAL(stream_global), + GRPC_CHTTP2_LIST_WRITABLE); } int grpc_chttp2_list_pop_writable_stream( @@ -159,20 +163,20 @@ int grpc_chttp2_list_pop_writable_stream( return r; } -void grpc_chttp2_list_remove_writable_stream( +bool grpc_chttp2_list_remove_writable_stream( grpc_chttp2_transport_global *transport_global, grpc_chttp2_stream_global *stream_global) { - stream_list_maybe_remove(TRANSPORT_FROM_GLOBAL(transport_global), - STREAM_FROM_GLOBAL(stream_global), - GRPC_CHTTP2_LIST_WRITABLE); + return stream_list_maybe_remove(TRANSPORT_FROM_GLOBAL(transport_global), + STREAM_FROM_GLOBAL(stream_global), + GRPC_CHTTP2_LIST_WRITABLE); } -int grpc_chttp2_list_add_writing_stream( +void grpc_chttp2_list_add_writing_stream( grpc_chttp2_transport_writing *transport_writing, grpc_chttp2_stream_writing *stream_writing) { - return stream_list_add(TRANSPORT_FROM_WRITING(transport_writing), - STREAM_FROM_WRITING(stream_writing), - GRPC_CHTTP2_LIST_WRITING); + GPR_ASSERT(stream_list_add(TRANSPORT_FROM_WRITING(transport_writing), + STREAM_FROM_WRITING(stream_writing), + GRPC_CHTTP2_LIST_WRITING)); } int grpc_chttp2_list_have_writing_streams( @@ -332,7 +336,7 @@ void grpc_chttp2_list_flush_writing_stalled_by_transport( while (stream_list_pop(transport, &stream, GRPC_CHTTP2_LIST_WRITING_STALLED_BY_TRANSPORT)) { if (is_window_available) { - grpc_chttp2_list_add_writable_stream(&transport->global, &stream->global); + grpc_chttp2_become_writable(&transport->global, &stream->global); } else { grpc_chttp2_list_add_stalled_by_transport(transport_writing, &stream->writing); diff --git a/src/core/transport/chttp2/writing.c b/src/core/transport/chttp2/writing.c index 356fd8174a7..107725cbc79 100644 --- a/src/core/transport/chttp2/writing.c +++ b/src/core/transport/chttp2/writing.c @@ -83,7 +83,8 @@ int grpc_chttp2_unlocking_check_writes( (according to available window sizes) and add to the output buffer */ while (grpc_chttp2_list_pop_writable_stream( transport_global, transport_writing, &stream_global, &stream_writing)) { - uint8_t sent_initial_metadata; + bool sent_initial_metadata = stream_writing->sent_initial_metadata; + bool become_writable = false; stream_writing->id = stream_global->id; stream_writing->read_closed = stream_global->read_closed; @@ -92,16 +93,12 @@ int grpc_chttp2_unlocking_check_writes( outgoing_window, stream_global, outgoing_window); - sent_initial_metadata = stream_writing->sent_initial_metadata; if (!sent_initial_metadata && stream_global->send_initial_metadata) { stream_writing->send_initial_metadata = stream_global->send_initial_metadata; stream_global->send_initial_metadata = NULL; - if (grpc_chttp2_list_add_writing_stream(transport_writing, - stream_writing)) { - GRPC_CHTTP2_STREAM_REF(stream_global, "chttp2_writing"); - } - sent_initial_metadata = 1; + become_writable = true; + sent_initial_metadata = true; } if (sent_initial_metadata) { if (stream_global->send_message != NULL) { @@ -128,10 +125,7 @@ int grpc_chttp2_unlocking_check_writes( stream_writing->flow_controlled_buffer.length > 0) && stream_writing->outgoing_window > 0) { if (transport_writing->outgoing_window > 0) { - if (grpc_chttp2_list_add_writing_stream(transport_writing, - stream_writing)) { - GRPC_CHTTP2_STREAM_REF(stream_global, "chttp2_writing"); - } + become_writable = true; } else { grpc_chttp2_list_add_stalled_by_transport(transport_writing, stream_writing); @@ -141,10 +135,7 @@ int grpc_chttp2_unlocking_check_writes( stream_writing->send_trailing_metadata = stream_global->send_trailing_metadata; stream_global->send_trailing_metadata = NULL; - if (grpc_chttp2_list_add_writing_stream(transport_writing, - stream_writing)) { - GRPC_CHTTP2_STREAM_REF(stream_global, "chttp2_writing"); - } + become_writable = true; } } @@ -153,10 +144,13 @@ int grpc_chttp2_unlocking_check_writes( GRPC_CHTTP2_FLOW_MOVE_STREAM("write", transport_global, stream_writing, announce_window, stream_global, unannounced_incoming_window_for_writing); - if (grpc_chttp2_list_add_writing_stream(transport_writing, - stream_writing)) { - GRPC_CHTTP2_STREAM_REF(stream_global, "chttp2_writing"); - } + become_writable = true; + } + + if (become_writable) { + grpc_chttp2_list_add_writing_stream(transport_writing, stream_writing); + } else { + GRPC_CHTTP2_STREAM_UNREF(exec_ctx, stream_global, "chttp2_writing"); } } @@ -310,10 +304,7 @@ static void finalize_outbuf(grpc_exec_ctx *exec_ctx, (stream_writing->send_message && !stream_writing->fetching)) && stream_writing->outgoing_window > 0) { if (transport_writing->outgoing_window > 0) { - if (grpc_chttp2_list_add_writing_stream(transport_writing, - stream_writing)) { - /* do nothing - already reffed */ - } + grpc_chttp2_list_add_writing_stream(transport_writing, stream_writing); } else { grpc_chttp2_list_add_writing_stalled_by_transport(transport_writing, stream_writing); diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index b9f511e9460..a1ca78d4c4f 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -142,7 +142,7 @@ static void incoming_byte_stream_update_flow_control( static void fail_pending_writes(grpc_exec_ctx *exec_ctx, grpc_chttp2_stream_global *stream_global); -/* +/******************************************************************************* * CONSTRUCTION/DESTRUCTION/REFCOUNTING */ @@ -521,7 +521,6 @@ static void destroy_stream(grpc_exec_ctx *exec_ctx, grpc_transport *gt, s->global.id) == NULL); } - grpc_chttp2_list_remove_writable_stream(&t->global, &s->global); grpc_chttp2_list_remove_unannounced_incoming_window_available(&t->global, &s->global); grpc_chttp2_list_remove_stalled_by_transport(&t->global, &s->global); @@ -583,7 +582,7 @@ grpc_chttp2_stream_parsing *grpc_chttp2_parsing_accept_stream( return &accepting->parsing; } -/* +/******************************************************************************* * LOCK MANAGEMENT */ @@ -611,10 +610,18 @@ static void unlock(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t) { GPR_TIMER_END("unlock", 0); } -/* +/******************************************************************************* * OUTPUT PROCESSING */ +void grpc_chttp2_become_writable(grpc_chttp2_transport_global *transport_global, + grpc_chttp2_stream_global *stream_global) { + if (!TRANSPORT_FROM_GLOBAL(transport_global)->closed && + grpc_chttp2_list_add_writable_stream(transport_global, stream_global)) { + GRPC_CHTTP2_STREAM_REF(stream_global, "chttp2_writing"); + } +} + static void push_setting(grpc_chttp2_transport *t, grpc_chttp2_setting_id id, uint32_t value) { const grpc_chttp2_setting_parameters *sp = @@ -732,7 +739,7 @@ static void maybe_start_some_streams( stream_global->id, STREAM_FROM_GLOBAL(stream_global)); stream_global->in_stream_map = 1; transport_global->concurrent_stream_count++; - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } /* cancel out streams that will never be started */ while (transport_global->next_stream_id >= MAX_CLIENT_STREAM_ID && @@ -821,7 +828,7 @@ static void perform_stream_op_locked( maybe_start_some_streams(exec_ctx, transport_global); } else { GPR_ASSERT(stream_global->id != 0); - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } else { grpc_chttp2_complete_closure_step( @@ -838,7 +845,7 @@ static void perform_stream_op_locked( exec_ctx, &stream_global->send_message_finished, 0); } else if (stream_global->id != 0) { stream_global->send_message = op->send_message; - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } @@ -858,7 +865,7 @@ static void perform_stream_op_locked( } else if (stream_global->id != 0) { /* TODO(ctiller): check if there's flow control for any outstanding bytes before going writable */ - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } @@ -999,7 +1006,7 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, } } -/* +/******************************************************************************* * INPUT PROCESSING */ @@ -1064,7 +1071,6 @@ static void remove_stream(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, if (!s) { s = grpc_chttp2_stream_map_delete(&t->new_stream_map, id); } - grpc_chttp2_list_remove_writable_stream(&t->global, &s->global); GPR_ASSERT(s); s->global.in_stream_map = 0; if (t->parsing.incoming_stream == &s->parsing) { @@ -1080,6 +1086,9 @@ static void remove_stream(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, if (grpc_chttp2_unregister_stream(t, s) && t->global.sent_goaway) { close_transport_locked(exec_ctx, t); } + if (grpc_chttp2_list_remove_writable_stream(&t->global, &s->global)) { + GRPC_CHTTP2_STREAM_UNREF(exec_ctx, &s->global, "chttp2_writing"); + } new_stream_count = grpc_chttp2_stream_map_size(&t->parsing_stream_map) + grpc_chttp2_stream_map_size(&t->new_stream_map); @@ -1331,7 +1340,7 @@ static void update_global_window(void *args, uint32_t id, void *stream) { is_zero = stream_global->outgoing_window <= 0; if (was_zero && !is_zero) { - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } @@ -1426,7 +1435,7 @@ static void recv_data(grpc_exec_ctx *exec_ctx, void *tp, bool success) { GPR_TIMER_END("recv_data", 0); } -/* +/******************************************************************************* * CALLBACK LOOP */ @@ -1440,7 +1449,7 @@ static void connectivity_state_set( state, reason); } -/* +/******************************************************************************* * POLLSET STUFF */ @@ -1468,7 +1477,7 @@ static void set_pollset(grpc_exec_ctx *exec_ctx, grpc_transport *gt, unlock(exec_ctx, t); } -/* +/******************************************************************************* * BYTE STREAM */ @@ -1508,7 +1517,7 @@ static void incoming_byte_stream_update_flow_control( add_max_recv_bytes); grpc_chttp2_list_add_unannounced_incoming_window_available(transport_global, stream_global); - grpc_chttp2_list_add_writable_stream(transport_global, stream_global); + grpc_chttp2_become_writable(transport_global, stream_global); } } @@ -1623,7 +1632,7 @@ grpc_chttp2_incoming_byte_stream *grpc_chttp2_incoming_byte_stream_create( return incoming_byte_stream; } -/* +/******************************************************************************* * TRACING */ @@ -1709,7 +1718,7 @@ void grpc_chttp2_flowctl_trace(const char *file, int line, const char *phase, gpr_free(prefix); } -/* +/******************************************************************************* * INTEGRATION GLUE */ diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c index 14912af7df1..807ae071a30 100644 --- a/src/core/transport/metadata.c +++ b/src/core/transport/metadata.c @@ -43,11 +43,13 @@ #include #include #include + #include "src/core/profiling/timers.h" #include "src/core/support/murmur_hash.h" #include "src/core/support/string.h" #include "src/core/transport/chttp2/bin_encoder.h" #include "src/core/transport/static_metadata.h" +#include "src/core/iomgr/iomgr_internal.h" /* There are two kinds of mdelem and mdstr instances. * Static instances are declared in static_metadata.{h,c} and @@ -227,6 +229,9 @@ void grpc_mdctx_global_shutdown(void) { if (shard->count != 0) { gpr_log(GPR_DEBUG, "WARNING: %d metadata elements were leaked", shard->count); + if (grpc_iomgr_abort_on_leaks()) { + abort(); + } } gpr_free(shard->elems); } @@ -237,6 +242,9 @@ void grpc_mdctx_global_shutdown(void) { if (shard->count != 0) { gpr_log(GPR_DEBUG, "WARNING: %d metadata strings were leaked", shard->count); + if (grpc_iomgr_abort_on_leaks()) { + abort(); + } } gpr_free(shard->strs); } diff --git a/src/core/transport/transport.c b/src/core/transport/transport.c index 6e154b629ab..3b555fa9338 100644 --- a/src/core/transport/transport.c +++ b/src/core/transport/transport.c @@ -45,7 +45,7 @@ void grpc_stream_ref(grpc_stream_refcount *refcount, const char *reason) { #else void grpc_stream_ref(grpc_stream_refcount *refcount) { #endif - gpr_ref(&refcount->refs); + gpr_ref_non_zero(&refcount->refs); } #ifdef GRPC_STREAM_REFCOUNT_DEBUG diff --git a/src/python/grpcio/grpc/_cython/imports.generated.c b/src/python/grpcio/grpc/_cython/imports.generated.c index 4b1860ce8c9..8bd6ae6372b 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.c +++ b/src/python/grpcio/grpc/_cython/imports.generated.c @@ -220,6 +220,7 @@ gpr_event_get_type gpr_event_get_import; gpr_event_wait_type gpr_event_wait_import; gpr_ref_init_type gpr_ref_init_import; gpr_ref_type gpr_ref_import; +gpr_ref_non_zero_type gpr_ref_non_zero_import; gpr_refn_type gpr_refn_import; gpr_unref_type gpr_unref_import; gpr_stats_init_type gpr_stats_init_import; @@ -485,6 +486,7 @@ void pygrpc_load_imports(HMODULE library) { gpr_event_wait_import = (gpr_event_wait_type) GetProcAddress(library, "gpr_event_wait"); gpr_ref_init_import = (gpr_ref_init_type) GetProcAddress(library, "gpr_ref_init"); gpr_ref_import = (gpr_ref_type) GetProcAddress(library, "gpr_ref"); + gpr_ref_non_zero_import = (gpr_ref_non_zero_type) GetProcAddress(library, "gpr_ref_non_zero"); gpr_refn_import = (gpr_refn_type) GetProcAddress(library, "gpr_refn"); gpr_unref_import = (gpr_unref_type) GetProcAddress(library, "gpr_unref"); gpr_stats_init_import = (gpr_stats_init_type) GetProcAddress(library, "gpr_stats_init"); diff --git a/src/python/grpcio/grpc/_cython/imports.generated.h b/src/python/grpcio/grpc/_cython/imports.generated.h index a395dce7d6a..b70dcccd17a 100644 --- a/src/python/grpcio/grpc/_cython/imports.generated.h +++ b/src/python/grpcio/grpc/_cython/imports.generated.h @@ -610,6 +610,9 @@ extern gpr_ref_init_type gpr_ref_init_import; typedef void(*gpr_ref_type)(gpr_refcount *r); extern gpr_ref_type gpr_ref_import; #define gpr_ref gpr_ref_import +typedef void(*gpr_ref_non_zero_type)(gpr_refcount *r); +extern gpr_ref_non_zero_type gpr_ref_non_zero_import; +#define gpr_ref_non_zero gpr_ref_non_zero_import typedef void(*gpr_refn_type)(gpr_refcount *r, int n); extern gpr_refn_type gpr_refn_import; #define gpr_refn gpr_refn_import diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 1af34d97fbc..56db4ec686b 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -220,6 +220,7 @@ gpr_event_get_type gpr_event_get_import; gpr_event_wait_type gpr_event_wait_import; gpr_ref_init_type gpr_ref_init_import; gpr_ref_type gpr_ref_import; +gpr_ref_non_zero_type gpr_ref_non_zero_import; gpr_refn_type gpr_refn_import; gpr_unref_type gpr_unref_import; gpr_stats_init_type gpr_stats_init_import; @@ -481,6 +482,7 @@ void grpc_rb_load_imports(HMODULE library) { gpr_event_wait_import = (gpr_event_wait_type) GetProcAddress(library, "gpr_event_wait"); gpr_ref_init_import = (gpr_ref_init_type) GetProcAddress(library, "gpr_ref_init"); gpr_ref_import = (gpr_ref_type) GetProcAddress(library, "gpr_ref"); + gpr_ref_non_zero_import = (gpr_ref_non_zero_type) GetProcAddress(library, "gpr_ref_non_zero"); gpr_refn_import = (gpr_refn_type) GetProcAddress(library, "gpr_refn"); gpr_unref_import = (gpr_unref_type) GetProcAddress(library, "gpr_unref"); gpr_stats_init_import = (gpr_stats_init_type) GetProcAddress(library, "gpr_stats_init"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 38aabfaca8f..b972f60fc35 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -610,6 +610,9 @@ extern gpr_ref_init_type gpr_ref_init_import; typedef void(*gpr_ref_type)(gpr_refcount *r); extern gpr_ref_type gpr_ref_import; #define gpr_ref gpr_ref_import +typedef void(*gpr_ref_non_zero_type)(gpr_refcount *r); +extern gpr_ref_non_zero_type gpr_ref_non_zero_import; +#define gpr_ref_non_zero gpr_ref_non_zero_import typedef void(*gpr_refn_type)(gpr_refcount *r, int n); extern gpr_refn_type gpr_refn_import; #define gpr_refn gpr_refn_import diff --git a/test/cpp/interop/reconnect_interop_client.cc b/test/cpp/interop/reconnect_interop_client.cc index 1f6b352db17..79a60cc8602 100644 --- a/test/cpp/interop/reconnect_interop_client.cc +++ b/test/cpp/interop/reconnect_interop_client.cc @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From d7f12e30a0050b3fecb72fe7ad558b4d837e140f Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 3 Mar 2016 10:08:31 -0800 Subject: [PATCH 04/17] Fix accept_stream being called post-channel deletion - Have the server clear the accept_stream callback prior to destroying the channel (required a small transport op protocol change) - Have the transport not enact transport ops until parsing is completed (prevents accept_stream from disappearing mid-parse) --- src/core/channel/client_channel.c | 2 +- src/core/channel/client_uchannel.c | 2 +- src/core/surface/server.c | 14 +++++++-- src/core/transport/chttp2/internal.h | 3 ++ src/core/transport/chttp2_transport.c | 43 +++++++++++++++++++-------- src/core/transport/transport.h | 6 ++-- 6 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/core/channel/client_channel.c b/src/core/channel/client_channel.c index eeac3c146c3..d4ba9508185 100644 --- a/src/core/channel/client_channel.c +++ b/src/core/channel/client_channel.c @@ -251,7 +251,7 @@ static void cc_start_transport_op(grpc_exec_ctx *exec_ctx, grpc_exec_ctx_enqueue(exec_ctx, op->on_consumed, true, NULL); - GPR_ASSERT(op->set_accept_stream == NULL); + GPR_ASSERT(op->set_accept_stream == false); if (op->bind_pollset != NULL) { grpc_pollset_set_add_pollset(exec_ctx, chand->interested_parties, op->bind_pollset); diff --git a/src/core/channel/client_uchannel.c b/src/core/channel/client_uchannel.c index b1e7155773f..83fcc3a87f5 100644 --- a/src/core/channel/client_uchannel.c +++ b/src/core/channel/client_uchannel.c @@ -107,7 +107,7 @@ static void cuc_start_transport_op(grpc_exec_ctx *exec_ctx, grpc_exec_ctx_enqueue(exec_ctx, op->on_consumed, true, NULL); - GPR_ASSERT(op->set_accept_stream == NULL); + GPR_ASSERT(op->set_accept_stream == false); GPR_ASSERT(op->bind_pollset == NULL); if (op->on_connectivity_state_change != NULL) { diff --git a/src/core/surface/server.c b/src/core/surface/server.c index fb5e0d4b9e7..5b13d4ba526 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -407,8 +407,15 @@ static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand) { maybe_finish_shutdown(exec_ctx, chand->server); chand->finish_destroy_channel_closure.cb = finish_destroy_channel; chand->finish_destroy_channel_closure.cb_arg = chand; - grpc_exec_ctx_enqueue(exec_ctx, &chand->finish_destroy_channel_closure, true, - NULL); + + grpc_transport_op op; + memset(&op, 0, sizeof(op)); + op.set_accept_stream = true; + op.on_consumed = &chand->finish_destroy_channel_closure; + grpc_channel_next_op(exec_ctx, + grpc_channel_stack_element( + grpc_channel_get_channel_stack(chand->channel), 0), + &op); } static void finish_start_new_rpc(grpc_exec_ctx *exec_ctx, grpc_server *server, @@ -971,7 +978,8 @@ void grpc_server_setup_transport(grpc_exec_ctx *exec_ctx, grpc_server *s, GRPC_CHANNEL_INTERNAL_REF(channel, "connectivity"); memset(&op, 0, sizeof(op)); - op.set_accept_stream = accept_stream; + op.set_accept_stream = true; + op.set_accept_stream_fn = accept_stream; op.set_accept_stream_user_data = chand; op.on_connectivity_state_change = &chand->channel_connectivity_changed; op.connectivity_state = &chand->connectivity_state; diff --git a/src/core/transport/chttp2/internal.h b/src/core/transport/chttp2/internal.h index 891aad6ef28..b720d1ab3e5 100644 --- a/src/core/transport/chttp2/internal.h +++ b/src/core/transport/chttp2/internal.h @@ -358,6 +358,9 @@ struct grpc_chttp2_transport { /** connectivity tracking */ grpc_connectivity_state_tracker state_tracker; } channel_callback; + + /** Transport op to be applied post-parsing */ + grpc_transport_op *post_parsing_op; }; typedef struct { diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index a1ca78d4c4f..369ff0ad7f6 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -951,12 +951,10 @@ void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, unlock(exec_ctx, t); } -static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, - grpc_transport_op *op) { - grpc_chttp2_transport *t = (grpc_chttp2_transport *)gt; - int close_transport = 0; - - lock(t); +static void perform_transport_op_locked(grpc_exec_ctx *exec_ctx, + grpc_chttp2_transport *t, + grpc_transport_op *op) { + bool close_transport = false; grpc_exec_ctx_enqueue(exec_ctx, op->on_consumed, true, NULL); @@ -975,8 +973,8 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, close_transport = !grpc_chttp2_has_streams(t); } - if (op->set_accept_stream != NULL) { - t->channel_callback.accept_stream = op->set_accept_stream; + if (op->set_accept_stream) { + t->channel_callback.accept_stream = op->set_accept_stream_fn; t->channel_callback.accept_stream_user_data = op->set_accept_stream_user_data; } @@ -997,15 +995,29 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, close_transport_locked(exec_ctx, t); } - unlock(exec_ctx, t); - if (close_transport) { - lock(t); close_transport_locked(exec_ctx, t); - unlock(exec_ctx, t); } } +static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, + grpc_transport_op *op) { + grpc_chttp2_transport *t = (grpc_chttp2_transport *)gt; + + lock(t); + + /* Let's be overly cautious: don't change any state while we're parsing */ + if (t->parsing_active) { + GPR_ASSERT(t->post_parsing_op == NULL); + t->post_parsing_op = gpr_malloc(sizeof(*op)); + memcpy(t->post_parsing_op, op, sizeof(*op)); + } else { + perform_transport_op_locked(exec_ctx, t, op); + } + + unlock(exec_ctx, t); +} + /******************************************************************************* * INPUT PROCESSING */ @@ -1401,6 +1413,13 @@ static void recv_data(grpc_exec_ctx *exec_ctx, void *tp, bool success) { /* handle higher level things */ grpc_chttp2_publish_reads(exec_ctx, transport_global, transport_parsing); t->parsing_active = 0; + /* handle delayed transport ops (if there is one) */ + if (t->post_parsing_op) { + grpc_transport_op *op = t->post_parsing_op; + t->post_parsing_op = NULL; + perform_transport_op_locked(exec_ctx, t, op); + gpr_free(op); + } /* if a stream is in the stream map, and gets cancelled, we need to ensure * we are not parsing before continuing the cancellation to keep things in * a sane state */ diff --git a/src/core/transport/transport.h b/src/core/transport/transport.h index 8902c5d2f65..7a007ef777b 100644 --- a/src/core/transport/transport.h +++ b/src/core/transport/transport.h @@ -139,8 +139,10 @@ typedef struct grpc_transport_op { gpr_slice *goaway_message; /** set the callback for accepting new streams; this is a permanent callback, unlike the other one-shot closures */ - void (*set_accept_stream)(grpc_exec_ctx *exec_ctx, void *user_data, - grpc_transport *transport, const void *server_data); + bool set_accept_stream; + void (*set_accept_stream_fn)(grpc_exec_ctx *exec_ctx, void *user_data, + grpc_transport *transport, + const void *server_data); void *set_accept_stream_user_data; /** add this transport to a pollset */ grpc_pollset *bind_pollset; From 66a6d014a4b2afec45906ff803e31dd89e82e22a Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 3 Mar 2016 12:35:08 -0800 Subject: [PATCH 05/17] run build_ext in build_python step --- tools/run_tests/build_python.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/run_tests/build_python.sh b/tools/run_tests/build_python.sh index 365abb4d863..f120fc7ed69 100755 --- a/tools/run_tests/build_python.sh +++ b/tools/run_tests/build_python.sh @@ -46,4 +46,5 @@ tox --notest $ROOT/.tox/py27/bin/python $ROOT/setup.py build $ROOT/.tox/py27/bin/python $ROOT/setup.py build_py +$ROOT/.tox/py27/bin/python $ROOT/setup.py build_ext --inplace $ROOT/.tox/py27/bin/python $ROOT/setup.py gather --test From 6dd3c707c2dd921288a82ec9813d41b567ff07b6 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 13:53:49 -0800 Subject: [PATCH 06/17] Update base INSTALL to markdown and remove outdated content --- CONTRIBUTING.md | 2 +- INSTALL | 217 ------------------------------ INSTALL.md | 45 +++++++ README.md | 2 +- examples/cpp/README.md | 2 +- examples/cpp/cpptutorial.md | 2 +- examples/cpp/helloworld/README.md | 2 +- 7 files changed, 50 insertions(+), 222 deletions(-) delete mode 100644 INSTALL create mode 100644 INSTALL.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9423c46547a..03d2e276a82 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,7 +13,7 @@ In order to protect both you and ourselves, you will need to sign the ### Technical requirements You will need several tools to work with this repository. In addition to all of -the packages described in the [INSTALL](INSTALL) file, you will also need +the packages described in the [INSTALL](INSTALL.md) file, you will also need python, and the mako template renderer. To install the latter, using pip, one should simply be able to do `pip install mako`. diff --git a/INSTALL b/INSTALL deleted file mode 100644 index e33f8970a95..00000000000 --- a/INSTALL +++ /dev/null @@ -1,217 +0,0 @@ -These instructions only cover building grpc C and C++ libraries under -typical unix systems. If you need more information, please try grpc's -wiki pages: - - https://github.com/google/grpc/wiki - - -************************* -* If you are in a hurry * -************************* - -On Linux (Debian): - - Note: you will need to add the Debian 'jessie-backports' distribution to your sources - file first. - - Add the following line to your `/etc/apt/sources.list` file: - - deb http://http.debian.net/debian jessie-backports main - - Install the gRPC library: - - $ [sudo] apt-get install libgrpc-dev - -OR - - $ git clone https://github.com/grpc/grpc.git - $ cd grpc - $ git submodule update --init - $ make - $ [sudo] make install - -You don't need anything else than GNU Make, gcc and autotools. Under a Debian -or Ubuntu system, this should boil down to the following packages: - - $ [sudo] apt-get install build-essential autoconf libtool - -Building the python wrapper requires the following: - - $ [sudo] apt-get install python-all-dev python-virtualenv - -If you want to install in a different directory than the default /usr/lib, you can -override it on the command line: - - $ [sudo] make install prefix=/opt - - -******************************* -* More detailled instructions * -******************************* - -Setting up dependencies -======================= - -Dependencies to compile the libraries -------------------------------------- - -grpc libraries have few external dependencies. If you need to compile and -install them, they are present in the third_party directory if you have -cloned the github repository recursively. If you didn't clone recursively, -you can still get them later by running the following command: - - $ git submodule update --init - -Note that the Makefile makes it much easier for you to compile from sources -if you were to clone recursively our git repository: it will automatically -compile zlib and OpenSSL, which are core requirements for grpc. Note this -creates grpc libraries that will have zlib and OpenSSL built-in inside of them, -which significantly increases the libraries' size. - -In order to decrease that size, you can manually install zlib and OpenSSL on -your system, so that the Makefile can use them instead. - -Under a Debian or Ubuntu system, one can acquire the development package -for zlib this way: - - # apt-get install zlib1g-dev - -To the best of our knowledge, no distribution has an OpenSSL package that -supports ALPN yet, so you would still have to depend on installing from source -for that particular dependency if you want to reduce the libraries' size. - -The recommended version of OpenSSL that provides ALPN support is available -at this URL: - - https://www.openssl.org/source/openssl-1.0.2.tar.gz - - -Dependencies to compile and run the tests ------------------------------------------ - -Compiling and running grpc plain-C tests dont't require any more dependency. - - -Compiling and running grpc C++ tests depend on protobuf 3.0.0, gtest and -gflags. Although gflags is provided in third_party, you will need to manually -install that dependency on your system to run these tests. - -Under a Debian or Ubuntu system, you can install the gtests and gflags packages -using apt-get: - - # apt-get install libgflags-dev libgtest-dev - -However, protobuf 3.0.0 isn't in a debian package yet, but the Makefile will -automatically try and compile the one present in third_party if you cloned the -repository recursively, and that it detects your system is lacking it. - -Compiling and installing protobuf 3.0.0 requires a few more dependencies in -itself, notably the autoconf suite. If you have apt-get, you can install -these dependencies this way: - - # apt-get install autoconf libtool - -If you want to run the tests using one of the sanitized configurations, you -will need clang and its instrumented libc++: - - # apt-get install clang libc++-dev - -Mac-specific notes: -------------------- - -For a Mac system, git is not available by default. You will first need to -install Xcode from the Mac AppStore and then run the following command from a -terminal: - - $ sudo xcode-select --install - -You should also install "port" following the instructions at -https://www.macports.org . This will reside in /opt/local/bin/port for -most Mac installations. Do the "git submodule" command listed above. - -Then execute the following for all the needed build dependencies - - $ sudo /opt/local/bin/port install autoconf automake libtool gflags cmake - $ mkdir ~/gtest-svn - $ svn checkout http://googletest.googlecode.com/svn/trunk/ gtest-svn - $ mkdir mybuild - $ cd mybuild - $ cmake ../gtest-svn - $ make - $ make gtest.a gtest_main.a - $ sudo cp libgtest.a libgtest_main.a /opt/local/lib - $ sudo mkdir /opt/local/include/gtest - $ sudo cp -pr ../gtest-svn/include/gtest /opt/local/include/gtest - -If you are going to make changes and need to regenerate the projects file, -you will need to install certain modules for python. - - $ sudo easy_install simplejson mako - -Mingw-specific notes: ---------------------- - -While gRPC compiles properly under mingw, some more preparation work is needed. -The recommendation is to use msys2. The installation instructions are available -at that address: http://msys2.github.io/ - -Once this is installed, make sure you are using the following: MinGW-w64 Win64. -You'll be required to install a few more packages: - - $ pacman -S make mingw-w64-x86_64-gcc mingw-w64-x86_64-zlib autoconf automake libtool - -Please also install OpenSSL from that website: - - http://slproweb.com/products/Win32OpenSSL.html - -The package Win64 OpenSSL v1.0.2a should do. At that point you should be able -to compile gRPC with the following: - - $ export LDFLAGS="-L/mingw64/lib -L/c/OpenSSL-Win64" - $ export CPPFLAGS="-I/mingw64/include -I/c/OpenSSL-Win64/include" - $ make - -A word on OpenSSL ------------------ - -Secure HTTP2 requires the TLS extension ALPN (see rfc 7301 and -http://http2.github.io/http2-spec/ section 3.3). Our HTTP2 implementation -relies on OpenSSL's implementation. OpenSSL 1.0.2 is the first released version -of OpenSSL that has ALPN support, and this explains our dependency on it. - -Note that the Makefile supports compiling only the unsecure elements of grpc, -and if you do not have OpenSSL and do not want it, you can still proceed -with installing only the elements you require. However, we strongly recommend -the use of encryption for all network traffic, and discourage the use of grpc -without TLS. - - -Compiling -========= - -If you have all the dependencies mentioned above, you should simply be able -to go ahead and run "make" to compile grpc's C and C++ libraries: - - $ make - - -Testing -======= - -To build and run the tests, you can run the command: - - $ make test - -If you want to be able to run them in parallel, and get better output, you can -also use the python tool we have written: - - $ ./tools/run_tests/run_tests.py - - -Installing -========== - -Once everything is compiled, you should be able to install grpc C and C++ -libraries and headers: - - # make install diff --git a/INSTALL.md b/INSTALL.md new file mode 100644 index 00000000000..aa809f23c8f --- /dev/null +++ b/INSTALL.md @@ -0,0 +1,45 @@ +#If you are in a hurry + +For language-specific installation instructions, please refer to these +documents + + * [C++](examples/cpp) + * [C#](src/csharp): NuGet package `Grpc` + * [Go](https://github.com/grpc/grpc-go): `go get google.golang.org/grpc` + * [Java](https://github.com/grpc/grpc-java) + * [Node](src/node): `npm install grpc` + * [Objective-C](src/objective-c) + * [PHP](src/php): `pecl install grpc-beta` + * [Python](src/python/grpcio): `pip install grpcio` + * [Ruby](src/ruby): `gem install grpc` + + +#Pre-requisites + +##Linux + +```sh + $ [sudo] apt-get install build-essential autoconf libtool +``` + +##Mac OSX + +For a Mac system, git is not available by default. You will first need to +install Xcode from the Mac AppStore and then run the following command from a +terminal: + +```sh + $ [sudo] xcode-select --install +``` + +#Build from Source + +This is for compiling just the gRPC C Core library. + +```sh + $ git clone https://github.com/grpc/grpc.git + $ cd grpc + $ git submodule update --init + $ make + $ [sudo] make install +``` diff --git a/README.md b/README.md index 033e09b91b7..abb4905392c 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ You can find more detailed documentation and examples in the [doc](doc) and [exa #Installation -See [grpc/INSTALL](INSTALL) for installation instructions for various platforms. +See [INSTALL](INSTALL.md) for installation instructions for various platforms. #Repository Structure & Status diff --git a/examples/cpp/README.md b/examples/cpp/README.md index 85c495099b7..979ba55dbb5 100644 --- a/examples/cpp/README.md +++ b/examples/cpp/README.md @@ -2,7 +2,7 @@ ## Installation -To install gRPC on your system, follow the instructions [here](../../INSTALL). +To install gRPC on your system, follow the instructions [here](../../INSTALL.md). ## Hello C++ gRPC! diff --git a/examples/cpp/cpptutorial.md b/examples/cpp/cpptutorial.md index cd1cddb1115..ef9ca99c0fe 100644 --- a/examples/cpp/cpptutorial.md +++ b/examples/cpp/cpptutorial.md @@ -91,7 +91,7 @@ message Point { Next we need to generate the gRPC client and server interfaces from our .proto service definition. We do this using the protocol buffer compiler `protoc` with a special gRPC C++ plugin. -For simplicity, we've provided a [makefile](route_guide/Makefile) that runs `protoc` for you with the appropriate plugin, input, and output (if you want to run this yourself, make sure you've installed protoc and followed the gRPC code [installation instructions](../../INSTALL) first): +For simplicity, we've provided a [makefile](route_guide/Makefile) that runs `protoc` for you with the appropriate plugin, input, and output (if you want to run this yourself, make sure you've installed protoc and followed the gRPC code [installation instructions](../../INSTALL.md) first): ```shell $ make route_guide.grpc.pb.cc route_guide.pb.cc diff --git a/examples/cpp/helloworld/README.md b/examples/cpp/helloworld/README.md index 90f3d6b1475..8e11f7cdf3e 100644 --- a/examples/cpp/helloworld/README.md +++ b/examples/cpp/helloworld/README.md @@ -2,7 +2,7 @@ ### Install gRPC Make sure you have installed gRPC on your system. Follow the instructions here: -[https://github.com/grpc/grpc/blob/master/INSTALL](../../../INSTALL). +[https://github.com/grpc/grpc/blob/master/INSTALL](../../../INSTALL.md). ### Get the tutorial source code From a22a50d61795c38522fe8b032603c63fcef7f93f Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 14:49:03 -0800 Subject: [PATCH 07/17] bit of text change --- INSTALL.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index aa809f23c8f..d9411db021d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -1,7 +1,7 @@ #If you are in a hurry -For language-specific installation instructions, please refer to these -documents +For language-specific installation instructions for gRPC runtime, please +refer to these documents * [C++](examples/cpp) * [C#](src/csharp): NuGet package `Grpc` @@ -34,7 +34,8 @@ terminal: #Build from Source -This is for compiling just the gRPC C Core library. +For developers who are interested to contribute, here is how to compile the +gRPC C Core library. ```sh $ git clone https://github.com/grpc/grpc.git From 19f703dbb7805b95bdc64d166a4e5fb36e2dd192 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 3 Mar 2016 15:33:29 -0800 Subject: [PATCH 08/17] increase timeout for interop tests --- tools/run_tests/run_interop_tests.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index df3ab90a839..1dc772a8565 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -60,6 +60,8 @@ _SKIP_COMPRESSION = ['large_compressed_unary', _SKIP_ADVANCED = ['custom_metadata', 'status_code_and_message', 'unimplemented_method'] +_TEST_TIMEOUT = 3*60 + class CXXLanguage: def __init__(self): @@ -459,7 +461,7 @@ def cloud_to_prod_jobspec(language, test_case, server_host_name, environ=environ, shortname='%s:%s:%s:%s' % (suite_name, server_host_name, language, test_case), - timeout_seconds=90, + timeout_seconds=_TEST_TIMEOUT, flake_retries=5 if args.allow_flakes else 0, timeout_retries=2 if args.allow_flakes else 0, kill_handler=_job_kill_handler) @@ -495,7 +497,7 @@ def cloud_to_cloud_jobspec(language, test_case, server_name, server_host, environ=environ, shortname='cloud_to_cloud:%s:%s_server:%s' % (language, server_name, test_case), - timeout_seconds=90, + timeout_seconds=_TEST_TIMEOUT, flake_retries=5 if args.allow_flakes else 0, timeout_retries=2 if args.allow_flakes else 0, kill_handler=_job_kill_handler) From 79a0f0611c6235fbf636c3e754860e6de28b4e3c Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 3 Mar 2016 17:18:48 -0800 Subject: [PATCH 09/17] Properly delete Node OpenSSL headers after downloading them --- tools/run_tests/build_node.bat | 10 +++++++++- tools/run_tests/pre_build_node.bat | 9 +-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/run_tests/build_node.bat b/tools/run_tests/build_node.bat index 6896bc1d1bb..3ddd0e73cf7 100644 --- a/tools/run_tests/build_node.bat +++ b/tools/run_tests/build_node.bat @@ -27,4 +27,12 @@ @rem (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE @rem OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -npm install --build-from-source \ No newline at end of file +call npm install --build-from-source + +@rem delete the redundant openssl headers +for /f "delims=v" %%v in ('node --version') do ( + rmdir "%HOMEDRIVE%%HOMEPATH%\.node-gyp\%%v\include\node\openssl" /S /Q +) + +@rem rebuild, because it probably failed the first time +call npm install --build-from-source \ No newline at end of file diff --git a/tools/run_tests/pre_build_node.bat b/tools/run_tests/pre_build_node.bat index 6e7cbe5d420..ffb4a09f15a 100644 --- a/tools/run_tests/pre_build_node.bat +++ b/tools/run_tests/pre_build_node.bat @@ -28,12 +28,5 @@ @rem OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. @rem Expire cache after 1 week -npm update --cache-min 604800 +call npm update --cache-min 604800 -npm install node-gyp-install -.\node_modules\.bin\node-gyp-install.cmd - -@rem delete the redundant openssl headers -for /f "delims=v" %%v in ('node --version') do ( - rmdir "%HOMEDRIVE%%HOMEPATH%\.node-gyp\%%v\include\node\openssl" /S /Q -) \ No newline at end of file From 4913ea06eb4093a2229b3009dedb02828af1edc2 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 3 Mar 2016 17:22:53 -0800 Subject: [PATCH 10/17] Ensure node and npm are in the path when running tests --- tools/run_tests/build_node.bat | 4 ++++ tools/run_tests/pre_build_node.bat | 2 ++ tools/run_tests/run_node.bat | 1 + 3 files changed, 7 insertions(+) diff --git a/tools/run_tests/build_node.bat b/tools/run_tests/build_node.bat index 3ddd0e73cf7..886af0610ff 100644 --- a/tools/run_tests/build_node.bat +++ b/tools/run_tests/build_node.bat @@ -27,6 +27,10 @@ @rem (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE @rem OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +set PATH=%PATH%;C:\Program Files\nodejs\;%APPDATA%\npm + +del /f /q BUILD || rmdir build /s /q + call npm install --build-from-source @rem delete the redundant openssl headers diff --git a/tools/run_tests/pre_build_node.bat b/tools/run_tests/pre_build_node.bat index ffb4a09f15a..a29456f9ed9 100644 --- a/tools/run_tests/pre_build_node.bat +++ b/tools/run_tests/pre_build_node.bat @@ -27,6 +27,8 @@ @rem (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE @rem OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +set PATH=%PATH%;C:\Program Files\nodejs\;%APPDATA%\npm + @rem Expire cache after 1 week call npm update --cache-min 604800 diff --git a/tools/run_tests/run_node.bat b/tools/run_tests/run_node.bat index 41777363568..0987fbee559 100644 --- a/tools/run_tests/run_node.bat +++ b/tools/run_tests/run_node.bat @@ -27,6 +27,7 @@ @rem (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE @rem OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +set PATH=%PATH%;C:\Program Files\nodejs\;%APPDATA%\npm set JUNIT_REPORT_PATH=src\node\report.xml set JUNIT_REPORT_STACK=1 .\node_modules\.bin\mocha.cmd --reporter mocha-jenkins-reporter --timeout 8000 src\node\test \ No newline at end of file From 77fae2784ed7f894dc557ee2004364f2bb1b6b74 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 19:39:56 -0800 Subject: [PATCH 11/17] fix broken links --- doc/interop-test-descriptions.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/interop-test-descriptions.md b/doc/interop-test-descriptions.md index e618e967ee0..3beb1d11f4a 100644 --- a/doc/interop-test-descriptions.md +++ b/doc/interop-test-descriptions.md @@ -2,9 +2,8 @@ Interoperability Test Case Descriptions ======================================= Client and server use -[test.proto](https://github.com/grpc/grpc/blob/master/test/proto/test.proto) -and the [gRPC over HTTP/2 v2 -protocol](https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md). +[test.proto](../src/proto/grpc/testing/test.proto) +and the [gRPC over HTTP/2 v2 protocol](./PROTOCOL-HTTP2.md). Client ------ From 3f5c5c7e18f6b09d85e39f310f214e90316cc0ef Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 19:46:13 -0800 Subject: [PATCH 12/17] fix broken link --- examples/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/README.md b/examples/README.md index 84ec80057e1..cd85417ca20 100644 --- a/examples/README.md +++ b/examples/README.md @@ -447,4 +447,4 @@ $ greeter_client ## Read more! - You can find links to language-specific tutorials, examples, and other docs in each language's [quick start](#quickstart). -- [gRPC Authentication Support](doc/grpc-auth-support.md) introduces authentication support in gRPC with supported mechanisms and examples. +- [gRPC Authentication Support](http://www.grpc.io/docs/guides/auth.html) introduces authentication support in gRPC with supported mechanisms and examples. From 5821a444c703757d4bacdf2d58a3045c6cfbb838 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 19:46:58 -0800 Subject: [PATCH 13/17] remove stray grpc_common reference --- examples/node/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/node/README.md b/examples/node/README.md index 7a2bc9794f3..c1ef6b05ab0 100644 --- a/examples/node/README.md +++ b/examples/node/README.md @@ -20,7 +20,7 @@ TRY IT! - Run the server ```sh - $ # from this directory (grpc_common/node). + $ # from this directory $ node ./greeter_server.js & ``` From f9946d579bc5b258261177b5ff87df22e7d49a07 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 3 Mar 2016 20:06:20 -0800 Subject: [PATCH 14/17] remove old stuff from tools/README.md --- tools/README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/README.md b/tools/README.md index a0c41eb79ff..df4c6ef7d25 100644 --- a/tools/README.md +++ b/tools/README.md @@ -1,7 +1,5 @@ buildgen: template renderer for our build system. -distpackages: script to generate debian packages. - distrib: scripts to distribute language-specific packages. dockerfile: Docker files to test gRPC. @@ -12,6 +10,4 @@ gce: scripts to help setup testing infrastructure on GCE. jenkins: support for running tests on Jenkins. -profile_analyzer: pretty printer for gRPC profiling data. - run_tests: scripts to run gRPC tests in parallel. From 389297dedfe384246d4831a4fc33e7af7b0200e5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 3 Mar 2016 21:02:54 -0800 Subject: [PATCH 15/17] Document some things --- src/core/transport/transport.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/transport/transport.h b/src/core/transport/transport.h index 7a007ef777b..5a90bd6d387 100644 --- a/src/core/transport/transport.h +++ b/src/core/transport/transport.h @@ -123,7 +123,8 @@ typedef struct grpc_transport_stream_op { /** Transport op: a set of operations to perform on a transport as a whole */ typedef struct grpc_transport_op { - /** called when processing of this op is done */ + /** Called when processing of this op is done. + Only one transport_op is allowed to be outstanding at any time. */ grpc_closure *on_consumed; /** connectivity monitoring - set connectivity_state to NULL to unsubscribe */ grpc_closure *on_connectivity_state_change; @@ -138,7 +139,9 @@ typedef struct grpc_transport_op { grpc_status_code goaway_status; gpr_slice *goaway_message; /** set the callback for accepting new streams; - this is a permanent callback, unlike the other one-shot closures */ + this is a permanent callback, unlike the other one-shot closures. + If true, the callback is set to set_accept_stream_fn, with its + user_data argument set to set_accept_stream_user_data */ bool set_accept_stream; void (*set_accept_stream_fn)(grpc_exec_ctx *exec_ctx, void *user_data, grpc_transport *transport, From 1bd9ea407fd673f6c795a524c6454bc5db339a85 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 3 Mar 2016 21:09:31 -0800 Subject: [PATCH 16/17] Refine condition --- src/core/transport/chttp2_transport.c | 5 +++-- src/core/transport/transport.h | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index 369ff0ad7f6..cf1dbfa0e59 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -1006,8 +1006,9 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, lock(t); - /* Let's be overly cautious: don't change any state while we're parsing */ - if (t->parsing_active) { + /* If there's a set_accept_stream ensure that we're not parsing + to avoid changing things out from underneath */ + if (t->parsing_active && t->set_accept_stream) { GPR_ASSERT(t->post_parsing_op == NULL); t->post_parsing_op = gpr_malloc(sizeof(*op)); memcpy(t->post_parsing_op, op, sizeof(*op)); diff --git a/src/core/transport/transport.h b/src/core/transport/transport.h index 5a90bd6d387..ed6e121c9cb 100644 --- a/src/core/transport/transport.h +++ b/src/core/transport/transport.h @@ -123,8 +123,7 @@ typedef struct grpc_transport_stream_op { /** Transport op: a set of operations to perform on a transport as a whole */ typedef struct grpc_transport_op { - /** Called when processing of this op is done. - Only one transport_op is allowed to be outstanding at any time. */ + /** Called when processing of this op is done. */ grpc_closure *on_consumed; /** connectivity monitoring - set connectivity_state to NULL to unsubscribe */ grpc_closure *on_connectivity_state_change; From 687bf6295f5e3a02f249aced69868e497c1bcc30 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 3 Mar 2016 21:18:48 -0800 Subject: [PATCH 17/17] Fix typo --- src/core/transport/chttp2_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index cf1dbfa0e59..643e3feeb97 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -1008,7 +1008,7 @@ static void perform_transport_op(grpc_exec_ctx *exec_ctx, grpc_transport *gt, /* If there's a set_accept_stream ensure that we're not parsing to avoid changing things out from underneath */ - if (t->parsing_active && t->set_accept_stream) { + if (t->parsing_active && op->set_accept_stream) { GPR_ASSERT(t->post_parsing_op == NULL); t->post_parsing_op = gpr_malloc(sizeof(*op)); memcpy(t->post_parsing_op, op, sizeof(*op));