You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2021/03/11 21:42:13 UTC

[thrift] branch master updated: THIRFT-5367 Fix crashes when using Ruby compaction GC Client: rb Patch: Stan Hu

This is an automated email from the ASF dual-hosted git repository.

jensg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new cc70b4e  THIRFT-5367 Fix crashes when using Ruby compaction GC Client: rb Patch: Stan Hu
cc70b4e is described below

commit cc70b4e89a1579559bc50fb8216c471a5c550926
Author: Stan Hu <st...@gmail.com>
AuthorDate: Thu Mar 11 03:49:57 2021 +0530

    THIRFT-5367 Fix crashes when using Ruby compaction GC
    Client: rb
    Patch: Stan Hu
    
    This closes #2347
---
 lib/rb/Rakefile               |  2 +-
 lib/rb/ext/compact_protocol.c |  1 +
 lib/rb/ext/struct.c           |  9 +++++++++
 lib/rb/ext/thrift_native.c    | 17 +++++++++++++++++
 lib/rb/spec/spec_helper.rb    |  5 +++++
 lib/rb/thrift.gemspec         |  3 ++-
 6 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/rb/Rakefile b/lib/rb/Rakefile
index 5e5e5ac..b509281 100644
--- a/lib/rb/Rakefile
+++ b/lib/rb/Rakefile
@@ -54,7 +54,7 @@ namespace :'gen-rb' do
 
   task :'flat_spec' do
     dir = File.dirname(__FILE__) + '/spec'
-    mkdir_p("#{dir}/gen-rb/flat")
+    FileUtils.mkdir_p("#{dir}/gen-rb/flat")
     sh THRIFT, '--gen', 'rb', '--recurse', '-out', "#{dir}/gen-rb/flat", "#{dir}/ThriftNamespacedSpec.thrift"
   end
 
diff --git a/lib/rb/ext/compact_protocol.c b/lib/rb/ext/compact_protocol.c
index c0f46b9..fab2170 100644
--- a/lib/rb/ext/compact_protocol.c
+++ b/lib/rb/ext/compact_protocol.c
@@ -567,6 +567,7 @@ VALUE rb_thrift_compact_proto_read_binary(VALUE self) {
 
 static void Init_constants() {
   thrift_compact_protocol_class = rb_const_get(thrift_module, rb_intern("CompactProtocol"));
+  rb_global_variable(&thrift_compact_protocol_class);
 
   VERSION = rb_num2ll(rb_const_get(thrift_compact_protocol_class, rb_intern("VERSION")));
   VERSION_MASK = rb_num2ll(rb_const_get(thrift_compact_protocol_class, rb_intern("VERSION_MASK")));
diff --git a/lib/rb/ext/struct.c b/lib/rb/ext/struct.c
index e3aa855..79cbabe 100644
--- a/lib/rb/ext/struct.c
+++ b/lib/rb/ext/struct.c
@@ -698,14 +698,23 @@ void Init_struct() {
   rb_define_method(struct_module, "read", rb_thrift_struct_read, 1);
 
   thrift_union_class = rb_const_get(thrift_module, rb_intern("Union"));
+  rb_global_variable(&thrift_union_class);
 
   rb_define_method(thrift_union_class, "write", rb_thrift_union_write, 1);
   rb_define_method(thrift_union_class, "read", rb_thrift_union_read, 1);
 
   setfield_id = rb_intern("@setfield");
+  rb_global_variable(&setfield_id);
+
   setvalue_id = rb_intern("@value");
+  rb_global_variable(&setvalue_id);
 
   to_s_method_id = rb_intern("to_s");
+  rb_global_variable(&to_s_method_id);
+
   name_to_id_method_id = rb_intern("name_to_id");
+  rb_global_variable(&name_to_id_method_id);
+
   sorted_field_ids_method_id = rb_intern("sorted_field_ids");
+  rb_global_variable(&sorted_field_ids_method_id);
 }
diff --git a/lib/rb/ext/thrift_native.c b/lib/rb/ext/thrift_native.c
index 3430b7c..d535454 100644
--- a/lib/rb/ext/thrift_native.c
+++ b/lib/rb/ext/thrift_native.c
@@ -112,10 +112,19 @@ VALUE protocol_exception_class;
 void Init_thrift_native() {
   // cached classes
   thrift_module = rb_const_get(rb_cObject, rb_intern("Thrift"));
+  rb_global_variable(&thrift_module);
+
   thrift_bytes_module = rb_const_get(thrift_module, rb_intern("Bytes"));
+  rb_global_variable(&thrift_bytes_module);
+
   thrift_types_module = rb_const_get(thrift_module, rb_intern("Types"));
+  rb_global_variable(&thrift_types_module);
+
   rb_cSet = rb_const_get(rb_cObject, rb_intern("Set"));
+  rb_global_variable(&rb_cSet);
+
   protocol_exception_class = rb_const_get(thrift_module, rb_intern("ProtocolException"));
+  rb_global_variable(&protocol_exception_class);
 
   // Init ttype constants
   TTYPE_BOOL = FIX2INT(rb_const_get(thrift_types_module, rb_intern("BOOL")));
@@ -194,6 +203,14 @@ void Init_thrift_native() {
   class_sym = ID2SYM(rb_intern("class"));
   binary_sym = ID2SYM(rb_intern("binary"));
 
+  rb_global_variable(&type_sym);
+  rb_global_variable(&name_sym);
+  rb_global_variable(&key_sym);
+  rb_global_variable(&value_sym);
+  rb_global_variable(&element_sym);
+  rb_global_variable(&class_sym);
+  rb_global_variable(&binary_sym);
+
   Init_struct();
   Init_binary_protocol_accelerated();
   Init_compact_protocol();
diff --git a/lib/rb/spec/spec_helper.rb b/lib/rb/spec/spec_helper.rb
index 5bf98d0..7c16507 100644
--- a/lib/rb/spec/spec_helper.rb
+++ b/lib/rb/spec/spec_helper.rb
@@ -62,3 +62,8 @@ end
 
 $:.unshift File.join(File.dirname(__FILE__), *%w[gen-rb/flat])
 
+if defined?(GC.verify_compaction_references) == 'method'
+  # This method was added in Ruby 3.0.0. Calling it this way asks the GC to
+  # move objects around, helping to find object movement bugs.
+  GC.verify_compaction_references(double_heap: true, toward: :empty)
+end
diff --git a/lib/rb/thrift.gemspec b/lib/rb/thrift.gemspec
index 7481a29..d0366a7 100644
--- a/lib/rb/thrift.gemspec
+++ b/lib/rb/thrift.gemspec
@@ -27,7 +27,7 @@ Gem::Specification.new do |s|
 
   s.require_paths = %w[lib ext]
 
-  s.add_development_dependency 'bundler',            '~> 1.11'
+  s.add_development_dependency 'bundler',            '>= 1.11'
   s.add_development_dependency 'pry',                '~> 0.11.3'
   s.add_development_dependency 'pry-byebug',         '~> 3.6'
   s.add_development_dependency 'pry-stack_explorer', '~> 0.4.9.2'
@@ -35,6 +35,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'rack-test',          '~> 0.8.3'
   s.add_development_dependency 'rake',               '~> 12.3'
   s.add_development_dependency 'rspec',              '~> 3.7'
+  s.add_development_dependency 'srv',                '~> 1.0'
   s.add_development_dependency 'thin',               '~> 1.7'
 end