From c2cd95393de9025fe418b25773aee3a6396e90a1 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 25 Oct 2024 12:36:53 +0200 Subject: [PATCH 1/5] Cleanup test_helper.rb --- test/json/test_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/json/test_helper.rb b/test/json/test_helper.rb index 4955a02c..e8bba16f 100644 --- a/test/json/test_helper.rb +++ b/test/json/test_helper.rb @@ -1,12 +1,12 @@ case ENV['JSON'] when 'pure' - $:.unshift File.join(__dir__, '../../lib') + $LOAD_PATH.unshift(File.expand_path('../../../lib', __FILE__)) require 'json/pure' when 'ext' - $:.unshift File.join(__dir__, '../../ext'), File.join(__dir__, '../../lib') + $LOAD_PATH.unshift(File.expand_path('../../../ext', __FILE__), File.expand_path('../../../lib', __FILE__)) require 'json/ext' else - $:.unshift File.join(__dir__, '../../ext'), File.join(__dir__, '../../lib') + $LOAD_PATH.unshift(File.expand_path('../../../ext', __FILE__), File.expand_path('../../../lib', __FILE__)) require 'json' end From a0cd1de12658e4d3ee671edfbfce384c70164508 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 25 Oct 2024 09:55:25 +0200 Subject: [PATCH 2/5] Workaround rubygems $LOAD_PATH bug Ref: https://github.com/ruby/json/issues/647 Ref: https://github.com/rubygems/rubygems/pull/6490 Older rubygems are executing `extconf.rb` with a broken `$LOAD_PATH` causing the `json` gem native extension to be loaded with the stdlib version of the `.rb` files. This fails with ``` json/common.rb:82:in `initialize': wrong number of arguments (given 1, expected 0) (ArgumentError) ``` Since this is just for `extconf.rb` we can probably just accept that extra argument and ignore it. The bug was fixed in rubygems 3.4.9 / 2023-03-20 --- ext/json/ext/generator/generator.c | 9 +++++++++ test/json/json_generator_test.rb | 24 ++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/ext/json/ext/generator/generator.c b/ext/json/ext/generator/generator.c index c35e86d9..6bc3d420 100644 --- a/ext/json/ext/generator/generator.c +++ b/ext/json/ext/generator/generator.c @@ -963,6 +963,12 @@ static VALUE cState_generate(VALUE self, VALUE obj) return result; } +static VALUE cState_initialize(int argc, VALUE *argv, VALUE self) +{ + rb_warn("The json gem extension was loaded with the stdlib ruby code. You should upgrade rubygems with `gem update --system`"); + return self; +} + /* * call-seq: initialize_copy(orig) * @@ -1408,6 +1414,9 @@ void Init_generator(void) cState = rb_define_class_under(mGenerator, "State", rb_cObject); rb_define_alloc_func(cState, cState_s_allocate); rb_define_singleton_method(cState, "from_state", cState_from_state_s, 1); + rb_define_method(cState, "initialize", cState_initialize, -1); + rb_define_alias(cState, "initialize", "initialize"); // avoid method redefinition warnings + rb_define_method(cState, "initialize_copy", cState_init_copy, 1); rb_define_method(cState, "indent", cState_indent, 0); rb_define_method(cState, "indent=", cState_indent_set, 1); diff --git a/test/json/json_generator_test.rb b/test/json/json_generator_test.rb index 7dc45e3a..53a04a35 100755 --- a/test/json/json_generator_test.rb +++ b/test/json/json_generator_test.rb @@ -261,19 +261,19 @@ def test_buffer_initial_length end def test_gc - if respond_to?(:assert_in_out_err) && !(RUBY_PLATFORM =~ /java/) - assert_in_out_err(%w[-rjson -Ilib -Iext], <<-EOS, [], []) - bignum_too_long_to_embed_as_string = 1234567890123456789012345 - expect = bignum_too_long_to_embed_as_string.to_s - GC.stress = true - - 10.times do |i| - tmp = bignum_too_long_to_embed_as_string.to_json - raise "'\#{expect}' is expected, but '\#{tmp}'" unless tmp == expect - end - EOS + pid = fork do + bignum_too_long_to_embed_as_string = 1234567890123456789012345 + expect = bignum_too_long_to_embed_as_string.to_s + GC.stress = true + + 10.times do |i| + tmp = bignum_too_long_to_embed_as_string.to_json + raise "#{expect}' is expected, but '#{tmp}'" unless tmp == expect + end end - end if GC.respond_to?(:stress=) + _, status = Process.waitpid2(pid) + assert_predicate status, :success? + end if GC.respond_to?(:stress=) && Process.respond_to?(:fork) def test_configure_using_configure_and_merge numbered_state = { From 7fd353198d590844fa093d684c584233aa4a9df0 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 25 Oct 2024 12:00:21 +0200 Subject: [PATCH 3/5] Workaround being loaded alongside a different `json_pure` version Fix: https://github.com/ruby/json/issues/646 Since both `json` and `json_pure` expose the same files, if the versions don't match, the native extension may be loaded with Ruby code that don't match and is incompatible. By doing the `require json/ext/generator/state` from C we ensure we're at least loading that. But this is a dirty workaround for the 2.7.x branch, we should find a better way to fully isolate the two gems. --- ext/json/ext/generator/generator.c | 2 ++ lib/json/ext.rb | 3 --- test/json/ractor_test.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/json/ext/generator/generator.c b/ext/json/ext/generator/generator.c index 6bc3d420..abeffeda 100644 --- a/ext/json/ext/generator/generator.c +++ b/ext/json/ext/generator/generator.c @@ -1507,4 +1507,6 @@ void Init_generator(void) usascii_encindex = rb_usascii_encindex(); utf8_encindex = rb_utf8_encindex(); binary_encindex = rb_ascii8bit_encindex(); + + rb_require("json/ext/generator/state"); } diff --git a/lib/json/ext.rb b/lib/json/ext.rb index 775e28a9..92ef61ea 100644 --- a/lib/json/ext.rb +++ b/lib/json/ext.rb @@ -15,9 +15,6 @@ module Ext else require 'json/ext/parser' require 'json/ext/generator' - unless RUBY_ENGINE == 'jruby' - require 'json/ext/generator/state' - end $DEBUG and warn "Using Ext extension for JSON." JSON.parser = Parser JSON.generator = Generator diff --git a/test/json/ractor_test.rb b/test/json/ractor_test.rb index e0116400..646baddf 100644 --- a/test/json/ractor_test.rb +++ b/test/json/ractor_test.rb @@ -9,7 +9,7 @@ class JSONInRactorTest < Test::Unit::TestCase def test_generate - assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + assert_separately(%w[-rjson -Ilib -Iext], "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; $VERBOSE = nil require "json" From e285489d55dce04fc354ea12da401879239692cd Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 25 Oct 2024 12:35:29 +0200 Subject: [PATCH 4/5] json_pure: fix ractor compatibility This actually never worked, because the test was always testing the ext version from the stdlib, never the pure version nor the current ext version. --- lib/json/pure/parser.rb | 29 +++++++++++++++-------------- test/json/ractor_test.rb | 28 +++++++++++++++++++--------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/lib/json/pure/parser.rb b/lib/json/pure/parser.rb index 3dafe830..b5c2b1ba 100644 --- a/lib/json/pure/parser.rb +++ b/lib/json/pure/parser.rb @@ -148,25 +148,26 @@ def convert_encoding(source) end # Unescape characters in strings. - UNESCAPE_MAP = Hash.new { |h, k| h[k] = k.chr } - UNESCAPE_MAP.update({ - ?" => '"', - ?\\ => '\\', - ?/ => '/', - ?b => "\b", - ?f => "\f", - ?n => "\n", - ?r => "\r", - ?t => "\t", - ?u => nil, - }) + # UNESCAPE_MAP = Hash.new { |h, k| puts; p [:k, k]; h[k] = k.chr } + UNESCAPE_MAP = { + '"' => '"', + '\\' => '\\', + '/' => '/', + 'b' => "\b", + 'f' => "\f", + 'n' => "\n", + 'r' => "\r", + 't' => "\t", + 'u' => nil, + }.freeze STR_UMINUS = ''.respond_to?(:-@) def parse_string if scan(STRING) return '' if self[1].empty? - string = self[1].gsub(%r((?:\\[\\bfnrt"/]|(?:\\u(?:[A-Fa-f\d]{4}))+|\\[\x20-\xff]))n) do |c| - if u = UNESCAPE_MAP[$&[1]] + string = self[1].gsub(%r{(?:\\[\\bfnrt"/]|(?:\\u(?:[A-Fa-f\d]{4}))+|\\[\x20-\xff])}n) do |c| + k = $&[1] + if u = UNESCAPE_MAP.fetch(k) { k.chr } u else # \uXXXX bytes = ''.b diff --git a/test/json/ractor_test.rb b/test/json/ractor_test.rb index 646baddf..f857c9a8 100644 --- a/test/json/ractor_test.rb +++ b/test/json/ractor_test.rb @@ -9,10 +9,7 @@ class JSONInRactorTest < Test::Unit::TestCase def test_generate - assert_separately(%w[-rjson -Ilib -Iext], "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) - begin; - $VERBOSE = nil - require "json" + pid = fork do r = Ractor.new do json = JSON.generate({ 'a' => 2, @@ -26,9 +23,22 @@ def test_generate }) JSON.parse(json) end - expected_json = '{"a":2,"b":3.141,"c":"c","d":[1,"b",3.14],"e":{"foo":"bar"},' + - '"g":"\\"\\u0000\\u001f","h":1000.0,"i":0.001}' - assert_equal(JSON.parse(expected_json), r.take) - end; + expected_json = JSON.parse('{"a":2,"b":3.141,"c":"c","d":[1,"b",3.14],"e":{"foo":"bar"},' + + '"g":"\\"\\u0000\\u001f","h":1000.0,"i":0.001}') + actual_json = r.take + + if expected_json == actual_json + exit 0 + else + puts "Expected:" + puts expected_json + puts "Acutual:" + puts actual_json + puts + exit 1 + end + end + _, status = Process.waitpid2(pid) + assert_predicate status, :success? end -end if defined?(Ractor) +end if defined?(Ractor) && Process.respond_to?(:fork) From 10981db1a462acf2f2dc3271f2ec9788b0734e1e Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 25 Oct 2024 12:46:50 +0200 Subject: [PATCH 5/5] Update CHANGES --- CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 665635bb..2893b2b1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,9 @@ # Changes +* Workaround a bug in 3.4.8 and older https://github.com/rubygems/rubygems/pull/6490. +* Workaround different versions of `json` and `json_pure` being loaded (not officially supported). +* Make `json_pure` Ractor compatible. + ### 2024-10-24 (2.7.3) * Numerous performance optimizations in `JSON.generate` and `JSON.dump` (up to 2 times faster).