From b5b62720c5ce22c7d479a76546858ec90dd213b2 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 20 Feb 2016 22:48:56 +0100 Subject: [PATCH 1/4] Fixing a reported ruby crash when using ruby_vm_at_exit. --- src/ruby/ext/grpc/rb_grpc.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/ruby/ext/grpc/rb_grpc.c b/src/ruby/ext/grpc/rb_grpc.c index eefdb93d673..1f887d69b5e 100644 --- a/src/ruby/ext/grpc/rb_grpc.c +++ b/src/ruby/ext/grpc/rb_grpc.c @@ -269,20 +269,10 @@ static void Init_grpc_time_consts() { id_tv_nsec = rb_intern("tv_nsec"); } -/* - TODO: find an alternative to ruby_vm_at_exit that is ok in Ruby 2.0 where - RUBY_TYPED_FREE_IMMEDIATELY is not defined. - - At the moment, registering a function using ruby_vm_at_exit segfaults in Ruby - 2.0. This is not an issue with the gRPC handler. More likely, this was an - in issue with 2.0 that got resolved in 2.1 and has not been backported. -*/ -#ifdef RUBY_TYPED_FREE_IMMEDIATELY -static void grpc_rb_shutdown(ruby_vm_t *vm) { - (void)vm; +static void grpc_rb_shutdown(VALUE val) { + (void)val; grpc_shutdown(); } -#endif /* Initialize the GRPC module structs */ @@ -308,10 +298,7 @@ void Init_grpc_c() { grpc_init(); -/* TODO: find alternative to ruby_vm_at_exit that is ok in Ruby 2.0 */ -#ifdef RUBY_TYPED_FREE_IMMEDIATELY - ruby_vm_at_exit(grpc_rb_shutdown); -#endif + rb_set_end_proc(grpc_rb_shutdown, Qnil); grpc_rb_mGRPC = rb_define_module("GRPC"); grpc_rb_mGrpcCore = rb_define_module_under(grpc_rb_mGRPC, "Core"); From 6f4f02cf9f8d0b1a8dfa15e3f903b19a02ac9fdf Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sun, 21 Feb 2016 01:30:56 +0100 Subject: [PATCH 2/4] Slightly better workaround given the circumstances. --- src/ruby/ext/grpc/rb_grpc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ruby/ext/grpc/rb_grpc.c b/src/ruby/ext/grpc/rb_grpc.c index 1f887d69b5e..7344e1467c8 100644 --- a/src/ruby/ext/grpc/rb_grpc.c +++ b/src/ruby/ext/grpc/rb_grpc.c @@ -269,8 +269,7 @@ static void Init_grpc_time_consts() { id_tv_nsec = rb_intern("tv_nsec"); } -static void grpc_rb_shutdown(VALUE val) { - (void)val; +static void grpc_rb_shutdown(void) { grpc_shutdown(); } @@ -298,7 +297,7 @@ void Init_grpc_c() { grpc_init(); - rb_set_end_proc(grpc_rb_shutdown, Qnil); + atexit(grpc_rb_shutdown); grpc_rb_mGRPC = rb_define_module("GRPC"); grpc_rb_mGrpcCore = rb_define_module_under(grpc_rb_mGRPC, "Core"); From cb90397216ea5d1eb82bc257e1263c9b606ed6f2 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sun, 21 Feb 2016 22:58:26 +0100 Subject: [PATCH 3/4] Adding the init / destruct of grpc for ruby in a gpr_once. --- src/ruby/ext/grpc/rb_grpc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ruby/ext/grpc/rb_grpc.c b/src/ruby/ext/grpc/rb_grpc.c index 7344e1467c8..996357c2389 100644 --- a/src/ruby/ext/grpc/rb_grpc.c +++ b/src/ruby/ext/grpc/rb_grpc.c @@ -289,15 +289,20 @@ VALUE sym_code = Qundef; VALUE sym_details = Qundef; VALUE sym_metadata = Qundef; +static gpr_once g_once_init = GPR_ONCE_INIT; + +static void grpc_ruby_once_init() { + grpc_init(); + atexit(grpc_rb_shutdown); +} + void Init_grpc_c() { if (!grpc_rb_load_core()) { rb_raise(rb_eLoadError, "Couldn't find or load gRPC's dynamic C core"); return; } - grpc_init(); - - atexit(grpc_rb_shutdown); + gpr_once_init(&g_once_init, grpc_ruby_once_init); grpc_rb_mGRPC = rb_define_module("GRPC"); grpc_rb_mGrpcCore = rb_define_module_under(grpc_rb_mGRPC, "Core"); From 88dc3c5d32f8d6c627983ddfb7f5dc66e3d614ea Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Mon, 22 Feb 2016 03:21:08 +0100 Subject: [PATCH 4/4] Adding an explanatory comment. --- src/ruby/ext/grpc/rb_grpc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/ruby/ext/grpc/rb_grpc.c b/src/ruby/ext/grpc/rb_grpc.c index 996357c2389..0f9b18fa212 100644 --- a/src/ruby/ext/grpc/rb_grpc.c +++ b/src/ruby/ext/grpc/rb_grpc.c @@ -302,6 +302,16 @@ void Init_grpc_c() { return; } + /* ruby_vm_at_exit doesn't seem to be working. It would crash once every + * blue moon, and some users are getting it repeatedly. See the discussions + * - https://github.com/grpc/grpc/pull/5337 + * - https://bugs.ruby-lang.org/issues/12095 + * + * In order to still be able to handle the (unlikely) situation where the + * extension is loaded by a first Ruby VM that is subsequently destroyed, + * then loaded again by another VM within the same process, we need to + * schedule our initialization and destruction only once. + */ gpr_once_init(&g_once_init, grpc_ruby_once_init); grpc_rb_mGRPC = rb_define_module("GRPC");