From 6c0618918345982d4f9460dcabd21b367f614ecc Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Mon, 21 Mar 2016 17:56:32 -0700 Subject: [PATCH 01/12] Use a not-broken setuptools --- tools/run_tests/build_artifact_python.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/run_tests/build_artifact_python.sh b/tools/run_tests/build_artifact_python.sh index 7ba04d75463..1f23f9fade8 100755 --- a/tools/run_tests/build_artifact_python.sh +++ b/tools/run_tests/build_artifact_python.sh @@ -35,15 +35,18 @@ cd $(dirname $0)/../.. if [ "$SKIP_PIP_INSTALL" == "" ] then pip install --upgrade six - pip install --upgrade setuptools + # There's a bug in newer versions of setuptools (see + # https://bitbucket.org/pypa/setuptools/issues/503/pkg_resources_vendorpackagingrequirementsi) + pip install --upgrade 'setuptools==18' pip install -rrequirements.txt fi +export GRPC_PYTHON_USE_CUSTOM_BDIST=0 +export GRPC_PYTHON_BUILD_WITH_CYTHON=1 + # Build the source distribution first because MANIFEST.in cannot override # exclusion of built shared objects among package resources (for some # inexplicable reason). -GRPC_PYTHON_USE_CUSTOM_BDIST=0 \ -GRPC_PYTHON_BUILD_WITH_CYTHON=1 \ ${SETARCH_CMD} python setup.py \ sdist @@ -51,15 +54,11 @@ ${SETARCH_CMD} python setup.py \ # and thus ought to be run in a shell command separate of others. Further, it # trashes the actual bdist_wheel output, so it should be run first so that # bdist_wheel may be run unmolested. -GRPC_PYTHON_USE_CUSTOM_BDIST=0 \ -GRPC_PYTHON_BUILD_WITH_CYTHON=1 \ ${SETARCH_CMD} python setup.py \ build_tagged_ext # Wheel has a bug where directories don't get excluded. # https://bitbucket.org/pypa/wheel/issues/99/cannot-exclude-directory -GRPC_PYTHON_USE_CUSTOM_BDIST=0 \ -GRPC_PYTHON_BUILD_WITH_CYTHON=1 \ ${SETARCH_CMD} python setup.py \ bdist_wheel From 3eaaf39efffc95327b4ff440fc4a412486f94311 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 14 Mar 2016 11:53:42 -0700 Subject: [PATCH 02/12] Properly mark proc used in call credentials for garbage collection --- src/ruby/ext/grpc/rb_call_credentials.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/ruby/ext/grpc/rb_call_credentials.c b/src/ruby/ext/grpc/rb_call_credentials.c index 2426f106a99..402becf6aac 100644 --- a/src/ruby/ext/grpc/rb_call_credentials.c +++ b/src/ruby/ext/grpc/rb_call_credentials.c @@ -57,6 +57,10 @@ typedef struct grpc_rb_call_credentials { /* Holder of ruby objects involved in contructing the credentials */ VALUE mark; + /* The proc called when getting the credentials. Same pointer as + wrapped->state */ + VALUE proc; + /* The actual credentials */ grpc_call_credentials *wrapped; } grpc_rb_call_credentials; @@ -164,11 +168,11 @@ static void grpc_rb_call_credentials_mark(void *p) { return; } wrapper = (grpc_rb_call_credentials *)p; - /* If it's not already cleaned up, mark the mark object */ if (wrapper->mark != Qnil) { rb_gc_mark(wrapper->mark); } + rb_gc_mark(wrapper->proc); } static rb_data_type_t grpc_rb_call_credentials_data_type = { @@ -187,6 +191,7 @@ static rb_data_type_t grpc_rb_call_credentials_data_type = { static VALUE grpc_rb_call_credentials_alloc(VALUE cls) { grpc_rb_call_credentials *wrapper = ALLOC(grpc_rb_call_credentials); wrapper->wrapped = NULL; + wrapper->proc = Qnil; wrapper->mark = Qnil; return TypedData_Wrap_Struct(cls, &grpc_rb_call_credentials_data_type, wrapper); } @@ -267,6 +272,7 @@ static VALUE grpc_rb_call_credentials_init(VALUE self, VALUE proc) { return Qnil; } + wrapper->proc = proc; wrapper->wrapped = creds; rb_ivar_set(self, id_callback, proc); From 05afaa838aad09ca65c8da5108492e63f15ae3f9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 14 Mar 2016 13:01:00 -0700 Subject: [PATCH 03/12] Add GC marking for composite credentials --- src/ruby/ext/grpc/rb_call_credentials.c | 32 ++++++++-------------- src/ruby/ext/grpc/rb_channel_credentials.c | 23 ++++++++-------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/ruby/ext/grpc/rb_call_credentials.c b/src/ruby/ext/grpc/rb_call_credentials.c index 402becf6aac..7c4b69f7f0d 100644 --- a/src/ruby/ext/grpc/rb_call_credentials.c +++ b/src/ruby/ext/grpc/rb_call_credentials.c @@ -50,17 +50,13 @@ * grpc_call_credentials */ static VALUE grpc_rb_cCallCredentials = Qnil; -/* grpc_rb_call_credentials wraps a grpc_call_credentials. It provides a peer - * ruby object, 'mark' to minimize copying when a credential is created from - * ruby. */ +/* grpc_rb_call_credentials wraps a grpc_call_credentials. It provides a mark + * object that is used to hold references to any objects used to create the + * credentials. */ typedef struct grpc_rb_call_credentials { /* Holder of ruby objects involved in contructing the credentials */ VALUE mark; - /* The proc called when getting the credentials. Same pointer as - wrapped->state */ - VALUE proc; - /* The actual credentials */ grpc_call_credentials *wrapped; } grpc_rb_call_credentials; @@ -150,13 +146,8 @@ static void grpc_rb_call_credentials_free(void *p) { return; } wrapper = (grpc_rb_call_credentials *)p; - - /* Delete the wrapped object if the mark object is Qnil, which indicates that - * no other object is the actual owner. */ - if (wrapper->wrapped != NULL && wrapper->mark == Qnil) { - grpc_call_credentials_release(wrapper->wrapped); - wrapper->wrapped = NULL; - } + grpc_call_credentials_release(wrapper->wrapped); + wrapper->wrapped = NULL; xfree(p); } @@ -168,11 +159,9 @@ static void grpc_rb_call_credentials_mark(void *p) { return; } wrapper = (grpc_rb_call_credentials *)p; - /* If it's not already cleaned up, mark the mark object */ if (wrapper->mark != Qnil) { rb_gc_mark(wrapper->mark); } - rb_gc_mark(wrapper->proc); } static rb_data_type_t grpc_rb_call_credentials_data_type = { @@ -191,7 +180,6 @@ static rb_data_type_t grpc_rb_call_credentials_data_type = { static VALUE grpc_rb_call_credentials_alloc(VALUE cls) { grpc_rb_call_credentials *wrapper = ALLOC(grpc_rb_call_credentials); wrapper->wrapped = NULL; - wrapper->proc = Qnil; wrapper->mark = Qnil; return TypedData_Wrap_Struct(cls, &grpc_rb_call_credentials_data_type, wrapper); } @@ -199,7 +187,7 @@ static VALUE grpc_rb_call_credentials_alloc(VALUE cls) { /* Creates a wrapping object for a given call credentials. This should only be * called with grpc_call_credentials objects that are not already associated * with any Ruby object */ -VALUE grpc_rb_wrap_call_credentials(grpc_call_credentials *c) { +VALUE grpc_rb_wrap_call_credentials(grpc_call_credentials *c, VALUE mark) { VALUE rb_wrapper; grpc_rb_call_credentials *wrapper; if (c == NULL) { @@ -209,6 +197,7 @@ VALUE grpc_rb_wrap_call_credentials(grpc_call_credentials *c) { TypedData_Get_Struct(rb_wrapper, grpc_rb_call_credentials, &grpc_rb_call_credentials_data_type, wrapper); wrapper->wrapped = c; + wrapper->mark = mark; return rb_wrapper; } @@ -272,7 +261,7 @@ static VALUE grpc_rb_call_credentials_init(VALUE self, VALUE proc) { return Qnil; } - wrapper->proc = proc; + wrapper->mark = proc; wrapper->wrapped = creds; rb_ivar_set(self, id_callback, proc); @@ -283,15 +272,18 @@ static VALUE grpc_rb_call_credentials_compose(int argc, VALUE *argv, VALUE self) { grpc_call_credentials *creds; grpc_call_credentials *other; + VALUE mark; if (argc == 0) { return self; } + mark = rb_ary_new(); creds = grpc_rb_get_wrapped_call_credentials(self); for (int i = 0; i < argc; i++) { + rb_ary_push(mark, argv[i]); other = grpc_rb_get_wrapped_call_credentials(argv[i]); creds = grpc_composite_call_credentials_create(creds, other, NULL); } - return grpc_rb_wrap_call_credentials(creds); + return grpc_rb_wrap_call_credentials(creds, mark); } void Init_grpc_call_credentials() { diff --git a/src/ruby/ext/grpc/rb_channel_credentials.c b/src/ruby/ext/grpc/rb_channel_credentials.c index 8c6fc3b7eb6..f6490843113 100644 --- a/src/ruby/ext/grpc/rb_channel_credentials.c +++ b/src/ruby/ext/grpc/rb_channel_credentials.c @@ -49,8 +49,8 @@ static VALUE grpc_rb_cChannelCredentials = Qnil; /* grpc_rb_channel_credentials wraps a grpc_channel_credentials. It provides a - * peer ruby object, 'mark' to minimize copying when a credential is - * created from ruby. */ + * mark object that is used to hold references to any objects used to create + * the credentials. */ typedef struct grpc_rb_channel_credentials { /* Holder of ruby objects involved in constructing the credentials */ VALUE mark; @@ -66,13 +66,8 @@ static void grpc_rb_channel_credentials_free(void *p) { return; }; wrapper = (grpc_rb_channel_credentials *)p; - - /* Delete the wrapped object if the mark object is Qnil, which indicates that - * no other object is the actual owner. */ - if (wrapper->wrapped != NULL && wrapper->mark == Qnil) { - grpc_channel_credentials_release(wrapper->wrapped); - wrapper->wrapped = NULL; - } + grpc_channel_credentials_release(wrapper->wrapped); + wrapper->wrapped = NULL; xfree(p); } @@ -85,7 +80,6 @@ static void grpc_rb_channel_credentials_mark(void *p) { } wrapper = (grpc_rb_channel_credentials *)p; - /* If it's not already cleaned up, mark the mark object */ if (wrapper->mark != Qnil) { rb_gc_mark(wrapper->mark); } @@ -114,7 +108,7 @@ static VALUE grpc_rb_channel_credentials_alloc(VALUE cls) { /* Creates a wrapping object for a given channel credentials. This should only * be called with grpc_channel_credentials objects that are not already * associated with any Ruby object. */ -VALUE grpc_rb_wrap_channel_credentials(grpc_channel_credentials *c) { +VALUE grpc_rb_wrap_channel_credentials(grpc_channel_credentials *c, VALUE mark) { VALUE rb_wrapper; grpc_rb_channel_credentials *wrapper; if (c == NULL) { @@ -124,6 +118,7 @@ VALUE grpc_rb_wrap_channel_credentials(grpc_channel_credentials *c) { TypedData_Get_Struct(rb_wrapper, grpc_rb_channel_credentials, &grpc_rb_channel_credentials_data_type, wrapper); wrapper->wrapped = c; + wrapper->mark = mark; return rb_wrapper; } @@ -222,11 +217,15 @@ static VALUE grpc_rb_channel_credentials_compose(int argc, VALUE *argv, VALUE self) { grpc_channel_credentials *creds; grpc_call_credentials *other; + VALUE mark; if (argc == 0) { return self; } + mark = rb_ary_new(); + rb_ary_push(mark, self); creds = grpc_rb_get_wrapped_channel_credentials(self); for (int i = 0; i < argc; i++) { + rb_ary_push(mark, argv[i]); other = grpc_rb_get_wrapped_call_credentials(argv[i]); creds = grpc_composite_channel_credentials_create(creds, other, NULL); if (creds == NULL) { @@ -234,7 +233,7 @@ static VALUE grpc_rb_channel_credentials_compose(int argc, VALUE *argv, "Failed to compose channel and call credentials"); } } - return grpc_rb_wrap_channel_credentials(creds); + return grpc_rb_wrap_channel_credentials(creds, mark); } void Init_grpc_channel_credentials() { From 8c9edc2a57badc215f0266ae770a973416ad4c62 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 14 Mar 2016 15:51:56 -0700 Subject: [PATCH 04/12] Make channels and calls properly mark references to their credentials --- src/ruby/ext/grpc/rb_call.c | 6 ++++++ src/ruby/ext/grpc/rb_channel.c | 22 ++++++++-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index af05ddf6e78..cd0aa6aaf20 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -72,6 +72,10 @@ static ID id_cq; * the flags used to create metadata from a Hash */ static ID id_flags; +/* id_credentials is the name of the hidden ivar that preserves the value + * of the credentials added to the call */ +static ID id_credentials; + /* id_input_md is the name of the hidden ivar that preserves the hash used to * create metadata, so that references to the strings it contains last as long * as the call the metadata is added to. */ @@ -299,6 +303,7 @@ static VALUE grpc_rb_call_set_credentials(VALUE self, VALUE credentials) { "grpc_call_set_credentials failed with %s (code=%d)", grpc_call_error_detail_of(err), err); } + rb_ivar_set(self, id_credentials, credentials); return Qnil; } @@ -859,6 +864,7 @@ void Init_grpc_call() { id_cq = rb_intern("__cq"); id_flags = rb_intern("__flags"); id_input_md = rb_intern("__input_md"); + id_credentials = rb_intern("__credentials"); /* Ids used in constructing the batch result. */ sym_send_message = ID2SYM(rb_intern("send_message")); diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c index 0e6badbdaf0..e1aaa539db3 100644 --- a/src/ruby/ext/grpc/rb_channel.c +++ b/src/ruby/ext/grpc/rb_channel.c @@ -70,11 +70,10 @@ static VALUE grpc_rb_cChannel = Qnil; /* Used during the conversion of a hash to channel args during channel setup */ static VALUE grpc_rb_cChannelArgs; -/* grpc_rb_channel wraps a grpc_channel. It provides a peer ruby object, - * 'mark' to minimize copying when a channel is created from ruby. */ +/* grpc_rb_channel wraps a grpc_channel. */ typedef struct grpc_rb_channel { - /* Holder of ruby objects involved in constructing the channel */ - VALUE mark; + VALUE credentials; + /* The actual channel */ grpc_channel *wrapped; } grpc_rb_channel; @@ -87,13 +86,8 @@ static void grpc_rb_channel_free(void *p) { }; ch = (grpc_rb_channel *)p; - /* Deletes the wrapped object if the mark object is Qnil, which indicates - * that no other object is the actual owner. */ - if (ch->wrapped != NULL && ch->mark == Qnil) { + if (ch->wrapped != NULL) { grpc_channel_destroy(ch->wrapped); - rb_warning("channel gc: destroyed the c channel"); - } else { - rb_warning("channel gc: did not destroy the c channel"); } xfree(p); @@ -106,8 +100,8 @@ static void grpc_rb_channel_mark(void *p) { return; } channel = (grpc_rb_channel *)p; - if (channel->mark != Qnil) { - rb_gc_mark(channel->mark); + if (channel->credentials != Qnil) { + rb_gc_mark(channel->credentials); } } @@ -125,7 +119,7 @@ static rb_data_type_t grpc_channel_data_type = { static VALUE grpc_rb_channel_alloc(VALUE cls) { grpc_rb_channel *wrapper = ALLOC(grpc_rb_channel); wrapper->wrapped = NULL; - wrapper->mark = Qnil; + wrapper->credentials = Qnil; return TypedData_Wrap_Struct(cls, &grpc_channel_data_type, wrapper); } @@ -162,6 +156,7 @@ static VALUE grpc_rb_channel_init(int argc, VALUE *argv, VALUE self) { } ch = grpc_insecure_channel_create(target_chars, &args, NULL); } else { + wrapper->credentials = credentials; creds = grpc_rb_get_wrapped_channel_credentials(credentials); ch = grpc_secure_channel_create(creds, target_chars, &args, NULL); } @@ -330,7 +325,6 @@ static VALUE grpc_rb_channel_destroy(VALUE self) { if (ch != NULL) { grpc_channel_destroy(ch); wrapper->wrapped = NULL; - wrapper->mark = Qnil; } return Qnil; From e17f6afd43c4c3b6def91b50c37f5ef18ab48eba Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 2 Mar 2016 16:39:13 -0800 Subject: [PATCH 05/12] fix windows leaks --- src/core/iomgr/iocp_windows.c | 14 +++++++++----- src/core/iomgr/iocp_windows.h | 11 +++++++++-- src/core/iomgr/tcp_server_windows.c | 3 +-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/core/iomgr/iocp_windows.c b/src/core/iomgr/iocp_windows.c index 807729708ea..fa87e5246b0 100644 --- a/src/core/iomgr/iocp_windows.c +++ b/src/core/iomgr/iocp_windows.c @@ -71,7 +71,8 @@ static DWORD deadline_to_millis_timeout(gpr_timespec deadline, timeout, gpr_time_from_nanos(GPR_NS_PER_MS - 1, GPR_TIMESPAN))); } -void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { +grpc_iocp_work_status grpc_iocp_work(grpc_exec_ctx *exec_ctx, + gpr_timespec deadline) { BOOL success; DWORD bytes = 0; DWORD flags = 0; @@ -84,14 +85,14 @@ void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { g_iocp, &bytes, &completion_key, &overlapped, deadline_to_millis_timeout(deadline, gpr_now(deadline.clock_type))); if (success == 0 && overlapped == NULL) { - return; + return GRPC_IOCP_WORK_TIMEOUT; } GPR_ASSERT(completion_key && overlapped); if (overlapped == &g_iocp_custom_overlap) { gpr_atm_full_fetch_add(&g_custom_events, -1); if (completion_key == (ULONG_PTR)&g_iocp_kick_token) { /* We were awoken from a kick. */ - return; + return GRPC_IOCP_WORK_KICK; } gpr_log(GPR_ERROR, "Unknown custom completion key."); abort(); @@ -121,6 +122,7 @@ void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline) { } gpr_mu_unlock(&socket->state_mu); grpc_exec_ctx_enqueue(exec_ctx, closure, true, NULL); + return GRPC_IOCP_WORK_WORK; } void grpc_iocp_init(void) { @@ -140,10 +142,12 @@ void grpc_iocp_kick(void) { void grpc_iocp_flush(void) { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_iocp_work_status work_status; do { - grpc_iocp_work(&exec_ctx, gpr_inf_past(GPR_CLOCK_MONOTONIC)); - } while (grpc_exec_ctx_flush(&exec_ctx)); + work_status = grpc_iocp_work(&exec_ctx, gpr_inf_past(GPR_CLOCK_MONOTONIC)); + } while (work_status == GRPC_IOCP_WORK_KICK || + grpc_exec_ctx_flush(&exec_ctx)); } void grpc_iocp_shutdown(void) { diff --git a/src/core/iomgr/iocp_windows.h b/src/core/iomgr/iocp_windows.h index 75f3ba84770..8b2b1aeb5c2 100644 --- a/src/core/iomgr/iocp_windows.h +++ b/src/core/iomgr/iocp_windows.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 @@ -38,7 +38,14 @@ #include "src/core/iomgr/socket_windows.h" -void grpc_iocp_work(grpc_exec_ctx *exec_ctx, gpr_timespec deadline); +typedef enum { + GRPC_IOCP_WORK_WORK, + GRPC_IOCP_WORK_TIMEOUT, + GRPC_IOCP_WORK_KICK +} grpc_iocp_work_status; + +grpc_iocp_work_status grpc_iocp_work(grpc_exec_ctx *exec_ctx, + gpr_timespec deadline); void grpc_iocp_init(void); void grpc_iocp_kick(void); void grpc_iocp_flush(void); diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index ce930b8f41d..a4abc5b9743 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -240,8 +240,7 @@ static void decrement_active_ports_and_notify(grpc_exec_ctx *exec_ctx, sp->shutting_down = 0; gpr_mu_lock(&sp->server->mu); GPR_ASSERT(sp->server->active_ports > 0); - if (0 == --sp->server->active_ports && - sp->server->shutdown_complete != NULL) { + if (0 == --sp->server->active_ports) { notify = 1; } gpr_mu_unlock(&sp->server->mu); From ca5a9aded31e7abfcb627b7e7508b8f26e328461 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 17 Mar 2016 17:22:34 -0700 Subject: [PATCH 06/12] correct handling for shadowed assemblies when loading native extension --- .../Grpc.Core/Internal/NativeExtension.cs | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/csharp/Grpc.Core/Internal/NativeExtension.cs b/src/csharp/Grpc.Core/Internal/NativeExtension.cs index 4c742ab6c3d..282816d51e0 100644 --- a/src/csharp/Grpc.Core/Internal/NativeExtension.cs +++ b/src/csharp/Grpc.Core/Internal/NativeExtension.cs @@ -32,6 +32,7 @@ #endregion using System; +using System.Globalization; using System.IO; using System.Reflection; @@ -99,14 +100,30 @@ namespace Grpc.Core.Internal // TODO: allow customizing path to native extension (possibly through exposing a GrpcEnvironment property). var libraryFlavor = string.Format("{0}_{1}", GetPlatformString(), GetArchitectureString()); - var fullPath = Path.Combine(GetExecutingAssemblyDirectory(), + var fullPath = Path.Combine(Path.GetDirectoryName(GetAssemblyPath()), NativeLibrariesDir, libraryFlavor, GetNativeLibraryFilename()); return new UnmanagedLibrary(fullPath); } - private static string GetExecutingAssemblyDirectory() + private static string GetAssemblyPath() { - return Path.GetDirectoryName(typeof(NativeExtension).GetTypeInfo().Assembly.Location); + var assembly = typeof(NativeExtension).GetTypeInfo().Assembly; + + // If assembly is shadowed (e.g. in a webapp), EscapedCodeBase is pointing + // to the original location of the assembly, and Location is pointing + // to the shadow copy. We care about the original location because + // the native dlls don't get shadowed. + var escapedCodeBase = assembly.EscapedCodeBase; + if (IsFileUri(escapedCodeBase)) + { + return new Uri(escapedCodeBase).LocalPath; + } + return assembly.Location; + } + + private static bool IsFileUri(string uri) + { + return uri.ToLowerInvariant().StartsWith(Uri.UriSchemeFile); } private static string GetPlatformString() From 2209d4ec1ad6ac0a18627cef4e54416df42af652 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 25 Mar 2016 11:54:47 -0700 Subject: [PATCH 07/12] make grpc_pollset_set_create for windows match the header --- src/core/iomgr/pollset_set_windows.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/iomgr/pollset_set_windows.c b/src/core/iomgr/pollset_set_windows.c index 3b8eca28e61..d17b65b13b1 100644 --- a/src/core/iomgr/pollset_set_windows.c +++ b/src/core/iomgr/pollset_set_windows.c @@ -37,7 +37,7 @@ #include "src/core/iomgr/pollset_set_windows.h" -grpc_pollset_set* grpc_pollset_set_create(pollset_set) { return NULL; } +grpc_pollset_set* grpc_pollset_set_create(void) { return NULL; } void grpc_pollset_set_destroy(grpc_pollset_set* pollset_set) {} From def177b7febd6f96eed31628144cba964f1bae77 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 25 Mar 2016 15:00:47 -0700 Subject: [PATCH 08/12] Don't set -ex for nvm functions --- test/distrib/node/run_distrib_test.sh | 2 +- tools/run_tests/build_artifact_node.sh | 2 +- tools/run_tests/build_node.sh | 2 +- tools/run_tests/build_package_node.sh | 2 +- tools/run_tests/pre_build_node.sh | 2 +- tools/run_tests/run_node.sh | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/distrib/node/run_distrib_test.sh b/test/distrib/node/run_distrib_test.sh index 9b8f15771b8..18b339399bb 100755 --- a/test/distrib/node/run_distrib_test.sh +++ b/test/distrib/node/run_distrib_test.sh @@ -30,11 +30,11 @@ NODE_VERSION=$1 source ~/.nvm/nvm.sh -set -ex cd $(dirname $0) nvm install $NODE_VERSION +set -ex npm install -g node-static diff --git a/tools/run_tests/build_artifact_node.sh b/tools/run_tests/build_artifact_node.sh index 6aa48245383..ef3476a0381 100755 --- a/tools/run_tests/build_artifact_node.sh +++ b/tools/run_tests/build_artifact_node.sh @@ -30,9 +30,9 @@ NODE_TARGET_ARCH=$1 source ~/.nvm/nvm.sh -set -ex nvm use 4 +set -ex cd $(dirname $0)/../.. diff --git a/tools/run_tests/build_node.sh b/tools/run_tests/build_node.sh index 9c4af071856..cbe0e31d2eb 100755 --- a/tools/run_tests/build_node.sh +++ b/tools/run_tests/build_node.sh @@ -31,9 +31,9 @@ NODE_VERSION=$1 source ~/.nvm/nvm.sh -set -ex nvm use $NODE_VERSION +set -ex CONFIG=${CONFIG:-opt} diff --git a/tools/run_tests/build_package_node.sh b/tools/run_tests/build_package_node.sh index aca90a37500..540c8263117 100755 --- a/tools/run_tests/build_package_node.sh +++ b/tools/run_tests/build_package_node.sh @@ -29,9 +29,9 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. source ~/.nvm/nvm.sh -set -ex nvm use 4 +set -ex cd $(dirname $0)/../.. diff --git a/tools/run_tests/pre_build_node.sh b/tools/run_tests/pre_build_node.sh index 11f46d60fc2..1f55df0b7bc 100755 --- a/tools/run_tests/pre_build_node.sh +++ b/tools/run_tests/pre_build_node.sh @@ -31,9 +31,9 @@ NODE_VERSION=$1 source ~/.nvm/nvm.sh -set -ex nvm use $NODE_VERSION +set -ex export GRPC_CONFIG=${CONFIG:-opt} diff --git a/tools/run_tests/run_node.sh b/tools/run_tests/run_node.sh index d33890068dd..b94dc3ec628 100755 --- a/tools/run_tests/run_node.sh +++ b/tools/run_tests/run_node.sh @@ -30,9 +30,9 @@ NODE_VERSION=$1 source ~/.nvm/nvm.sh -set -ex nvm use $NODE_VERSION +set -ex CONFIG=${CONFIG:-opt} From f22f4c60bad22746b4e76928f82a3e35872f63e6 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 25 Mar 2016 15:52:42 -0700 Subject: [PATCH 09/12] Node distrib test: don't start all static servers on the same port --- test/distrib/node/run_distrib_test.sh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/distrib/node/run_distrib_test.sh b/test/distrib/node/run_distrib_test.sh index 18b339399bb..e9183a203b1 100755 --- a/test/distrib/node/run_distrib_test.sh +++ b/test/distrib/node/run_distrib_test.sh @@ -38,13 +38,10 @@ set -ex npm install -g node-static -# Kill off existing static servers -kill -9 $(ps aux | grep '[n]ode .*static' | awk '{print $2}') || true - STATIC_SERVER=127.0.0.1 -STATIC_PORT=8080 +STATIC_PORT=$$ -# Serves the input_artifacts directory statically at localhost:8080 +# Serves the input_artifacts directory statically at localhost: static "$EXTERNAL_GIT_ROOT/input_artifacts" -a $STATIC_SERVER -p $STATIC_PORT & STATIC_PID=$! From bebf2df6e4c2548ef53ec93a6339900ad3de61e5 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Mon, 28 Mar 2016 11:31:51 -0700 Subject: [PATCH 10/12] Make distribtest more robust, especially on Mac --- test/distrib/node/run_distrib_test.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/distrib/node/run_distrib_test.sh b/test/distrib/node/run_distrib_test.sh index e9183a203b1..13a42fcb0a2 100755 --- a/test/distrib/node/run_distrib_test.sh +++ b/test/distrib/node/run_distrib_test.sh @@ -28,6 +28,15 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +function finish() { + rv=$? + kill $STATIC_PID || true + curl "localhost:32767/drop/$STATIC_PORT" || true + exit $rv +} + +trap finish EXIT + NODE_VERSION=$1 source ~/.nvm/nvm.sh @@ -39,7 +48,9 @@ set -ex npm install -g node-static STATIC_SERVER=127.0.0.1 -STATIC_PORT=$$ +# If port_server is running, get port from that. Otherwise, assume we're in +# docker and use 8080 +STATIC_PORT=$(curl 'localhost:32767/get' || echo '8080') # Serves the input_artifacts directory statically at localhost: static "$EXTERNAL_GIT_ROOT/input_artifacts" -a $STATIC_SERVER -p $STATIC_PORT & @@ -49,6 +60,4 @@ STATIC_URL="http://$STATIC_SERVER:$STATIC_PORT/" npm install --unsafe-perm $STATIC_URL/grpc.tgz --grpc_node_binary_host_mirror=$STATIC_URL -kill -9 $STATIC_PID - ./distrib_test.js From 97daf35addf51951748d2954580fe10dffd3be36 Mon Sep 17 00:00:00 2001 From: Zhuochun Date: Sun, 13 Mar 2016 16:19:56 +0800 Subject: [PATCH 11/12] Fix Ruby memory leaks - `recv_message` is not cleaned up after call - `send_message` is not cleaned up in failure cases - `xmalloc` ruby string is not freed after used --- src/ruby/ext/grpc/rb_byte_buffer.c | 7 +++++-- src/ruby/ext/grpc/rb_call.c | 14 +++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ruby/ext/grpc/rb_byte_buffer.c b/src/ruby/ext/grpc/rb_byte_buffer.c index db7cac363a3..52523b8319a 100644 --- a/src/ruby/ext/grpc/rb_byte_buffer.c +++ b/src/ruby/ext/grpc/rb_byte_buffer.c @@ -52,12 +52,12 @@ grpc_byte_buffer* grpc_rb_s_to_byte_buffer(char *string, size_t length) { VALUE grpc_rb_byte_buffer_to_s(grpc_byte_buffer *buffer) { size_t length = 0; char *string = NULL; + VALUE rb_string; size_t offset = 0; grpc_byte_buffer_reader reader; gpr_slice next; if (buffer == NULL) { return Qnil; - } length = grpc_byte_buffer_length(buffer); string = xmalloc(length + 1); @@ -65,6 +65,9 @@ VALUE grpc_rb_byte_buffer_to_s(grpc_byte_buffer *buffer) { while (grpc_byte_buffer_reader_next(&reader, &next) != 0) { memcpy(string + offset, GPR_SLICE_START_PTR(next), GPR_SLICE_LENGTH(next)); offset += GPR_SLICE_LENGTH(next); + gpr_slice_unref(next); } - return rb_str_new(string, length); + rb_string = rb_str_new(string, length); + xfree(string); + return rb_string; } diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index cd0aa6aaf20..b0829efdc78 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -551,13 +551,26 @@ static void grpc_run_batch_stack_init(run_batch_stack *st, /* grpc_run_batch_stack_cleanup ensures the run_batch_stack is properly * cleaned up */ static void grpc_run_batch_stack_cleanup(run_batch_stack *st) { + size_t i = 0; + grpc_metadata_array_destroy(&st->send_metadata); grpc_metadata_array_destroy(&st->send_trailing_metadata); grpc_metadata_array_destroy(&st->recv_metadata); grpc_metadata_array_destroy(&st->recv_trailing_metadata); + if (st->recv_status_details != NULL) { gpr_free(st->recv_status_details); } + + if (st->recv_message != NULL) { + grpc_byte_buffer_destroy(st->recv_message); + } + + for (i = 0; i < st->op_num; i++) { + if (st->ops[i].op == GRPC_OP_SEND_MESSAGE) { + grpc_byte_buffer_destroy(st->ops[i].data.send_message); + } + } } /* grpc_run_batch_stack_fill_ops fills the run_batch_stack ops array from @@ -643,7 +656,6 @@ static VALUE grpc_run_batch_stack_build_result(run_batch_stack *st) { break; case GRPC_OP_SEND_MESSAGE: rb_struct_aset(result, sym_send_message, Qtrue); - grpc_byte_buffer_destroy(st->ops[i].data.send_message); break; case GRPC_OP_SEND_CLOSE_FROM_CLIENT: rb_struct_aset(result, sym_send_close, Qtrue); From 375d1f47cad2f678da49e3e3d0a5e6c79c5e0f30 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 30 Mar 2016 01:53:01 +0200 Subject: [PATCH 12/12] Better memory management for grpc_rb_byte_buffer_to_s. --- src/ruby/ext/grpc/rb_byte_buffer.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/ruby/ext/grpc/rb_byte_buffer.c b/src/ruby/ext/grpc/rb_byte_buffer.c index 52523b8319a..9b617e13d3c 100644 --- a/src/ruby/ext/grpc/rb_byte_buffer.c +++ b/src/ruby/ext/grpc/rb_byte_buffer.c @@ -50,24 +50,18 @@ grpc_byte_buffer* grpc_rb_s_to_byte_buffer(char *string, size_t length) { } VALUE grpc_rb_byte_buffer_to_s(grpc_byte_buffer *buffer) { - size_t length = 0; - char *string = NULL; VALUE rb_string; - size_t offset = 0; grpc_byte_buffer_reader reader; gpr_slice next; if (buffer == NULL) { return Qnil; } - length = grpc_byte_buffer_length(buffer); - string = xmalloc(length + 1); + rb_string = rb_str_buf_new(grpc_byte_buffer_length(buffer)); grpc_byte_buffer_reader_init(&reader, buffer); while (grpc_byte_buffer_reader_next(&reader, &next) != 0) { - memcpy(string + offset, GPR_SLICE_START_PTR(next), GPR_SLICE_LENGTH(next)); - offset += GPR_SLICE_LENGTH(next); + rb_str_cat(rb_string, (const char *) GPR_SLICE_START_PTR(next), + GPR_SLICE_LENGTH(next)); gpr_slice_unref(next); } - rb_string = rb_str_new(string, length); - xfree(string); return rb_string; }