From 065c48cdf11a1c4cece84db44ed8624d294f8fd5 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 16 Dec 2025 12:51:45 +0900 Subject: [PATCH 1/9] Revert "[Feature #6012] Extend `source_location` for end position and columns" This reverts commit 073c4e1cc712064e626914fa4a5a8061f903a637. https://bugs.ruby-lang.org/issues/6012#note-31 > we will cancel this feature in 4.0 because of design ambiguities > such as whether to return column positions in bytes or characters as > in [#21783]. [#21783]: https://bugs.ruby-lang.org/issues/21783 --- NEWS.md | 10 ---- proc.c | 16 ++---- spec/ruby/core/method/source_location_spec.rb | 16 ++---- spec/ruby/core/proc/source_location_spec.rb | 51 +++++++------------ .../unboundmethod/source_location_spec.rb | 18 +++---- test/ruby/test_lambda.rb | 12 ++--- test/ruby/test_proc.rb | 18 +++---- 7 files changed, 48 insertions(+), 93 deletions(-) diff --git a/NEWS.md b/NEWS.md index dbf44b63642720..4b16fd50189d0b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -116,15 +116,6 @@ Note: We're only listing outstanding class updates. * `Math.log1p` and `Math.expm1` are added. [[Feature #21527]] -* Method - - * `Method#source_location`, `Proc#source_location`, and - `UnboundMethod#source_location` now return extended location - information with 5 elements: `[path, start_line, start_column, - end_line, end_column]`. The previous 2-element format `[path, - line]` can still be obtained by calling `.take(2)` on the result. - [[Feature #6012]] - * Proc * `Proc#parameters` now shows anonymous optional parameters as `[:opt]` @@ -443,7 +434,6 @@ A lot of work has gone into making Ractors more stable, performant, and usable. * `--rjit` is removed. We will move the implementation of the third-party JIT API to the [ruby/rjit](https://github.com/ruby/rjit) repository. -[Feature #6012]: https://bugs.ruby-lang.org/issues/6012 [Feature #15408]: https://bugs.ruby-lang.org/issues/15408 [Feature #17473]: https://bugs.ruby-lang.org/issues/17473 [Feature #18455]: https://bugs.ruby-lang.org/issues/18455 diff --git a/proc.c b/proc.c index 9cd4c5b0c9f488..cdc453f50092d2 100644 --- a/proc.c +++ b/proc.c @@ -1511,20 +1511,14 @@ proc_eq(VALUE self, VALUE other) static VALUE iseq_location(const rb_iseq_t *iseq) { - VALUE loc[5]; - int i = 0; + VALUE loc[2]; if (!iseq) return Qnil; rb_iseq_check(iseq); - loc[i++] = rb_iseq_path(iseq); - const rb_code_location_t *cl = &ISEQ_BODY(iseq)->location.code_location; - loc[i++] = RB_INT2NUM(cl->beg_pos.lineno); - loc[i++] = RB_INT2NUM(cl->beg_pos.column); - loc[i++] = RB_INT2NUM(cl->end_pos.lineno); - loc[i++] = RB_INT2NUM(cl->end_pos.column); - RUBY_ASSERT_ALWAYS(i == numberof(loc)); - - return rb_ary_new_from_values(i, loc); + loc[0] = rb_iseq_path(iseq); + loc[1] = RB_INT2NUM(ISEQ_BODY(iseq)->location.first_lineno); + + return rb_ary_new4(2, loc); } VALUE diff --git a/spec/ruby/core/method/source_location_spec.rb b/spec/ruby/core/method/source_location_spec.rb index 1b175ebabac044..c5b296f6e2a7b0 100644 --- a/spec/ruby/core/method/source_location_spec.rb +++ b/spec/ruby/core/method/source_location_spec.rb @@ -11,23 +11,23 @@ end it "sets the first value to the path of the file in which the method was defined" do - file = @method.source_location[0] + file = @method.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/classes.rb', __dir__) end it "sets the last value to an Integer representing the line on which the method was defined" do - line = @method.source_location[1] + line = @method.source_location.last line.should be_an_instance_of(Integer) line.should == 5 end it "returns the last place the method was defined" do - MethodSpecs::SourceLocation.method(:redefined).source_location[1].should == 13 + MethodSpecs::SourceLocation.method(:redefined).source_location.last.should == 13 end it "returns the location of the original method even if it was aliased" do - MethodSpecs::SourceLocation.new.method(:aka).source_location[1].should == 17 + MethodSpecs::SourceLocation.new.method(:aka).source_location.last.should == 17 end it "works for methods defined with a block" do @@ -108,13 +108,7 @@ def f c = Class.new do eval('def self.m; end', nil, "foo", 100) end - location = c.method(:m).source_location - ruby_version_is(""..."4.0") do - location.should == ["foo", 100] - end - ruby_version_is("4.0") do - location.should == ["foo", 100, 0, 100, 15] - end + c.method(:m).source_location.should == ["foo", 100] end describe "for a Method generated by respond_to_missing?" do diff --git a/spec/ruby/core/proc/source_location_spec.rb b/spec/ruby/core/proc/source_location_spec.rb index 69b4e2fd8273b6..a8b99287d5c380 100644 --- a/spec/ruby/core/proc/source_location_spec.rb +++ b/spec/ruby/core/proc/source_location_spec.rb @@ -17,64 +17,57 @@ end it "sets the first value to the path of the file in which the proc was defined" do - file = @proc.source_location[0] + file = @proc.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @proc_new.source_location[0] + file = @proc_new.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @lambda.source_location[0] + file = @lambda.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) - file = @method.source_location[0] + file = @method.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/source_location.rb', __dir__) end - it "sets the second value to an Integer representing the line on which the proc was defined" do - line = @proc.source_location[1] + it "sets the last value to an Integer representing the line on which the proc was defined" do + line = @proc.source_location.last line.should be_an_instance_of(Integer) line.should == 4 - line = @proc_new.source_location[1] + line = @proc_new.source_location.last line.should be_an_instance_of(Integer) line.should == 12 - line = @lambda.source_location[1] + line = @lambda.source_location.last line.should be_an_instance_of(Integer) line.should == 8 - line = @method.source_location[1] + line = @method.source_location.last line.should be_an_instance_of(Integer) line.should == 15 end it "works even if the proc was created on the same line" do - ruby_version_is(""..."4.0") do - proc { true }.source_location.should == [__FILE__, __LINE__] - Proc.new { true }.source_location.should == [__FILE__, __LINE__] - -> { true }.source_location.should == [__FILE__, __LINE__] - end - ruby_version_is("4.0") do - proc { true }.source_location.should == [__FILE__, __LINE__, 11, __LINE__, 19] - Proc.new { true }.source_location.should == [__FILE__, __LINE__, 15, __LINE__, 23] - -> { true }.source_location.should == [__FILE__, __LINE__, 8, __LINE__, 17] - end + proc { true }.source_location.should == [__FILE__, __LINE__] + Proc.new { true }.source_location.should == [__FILE__, __LINE__] + -> { true }.source_location.should == [__FILE__, __LINE__] end it "returns the first line of a multi-line proc (i.e. the line containing 'proc do')" do - ProcSpecs::SourceLocation.my_multiline_proc.source_location[1].should == 20 - ProcSpecs::SourceLocation.my_multiline_proc_new.source_location[1].should == 34 - ProcSpecs::SourceLocation.my_multiline_lambda.source_location[1].should == 27 + ProcSpecs::SourceLocation.my_multiline_proc.source_location.last.should == 20 + ProcSpecs::SourceLocation.my_multiline_proc_new.source_location.last.should == 34 + ProcSpecs::SourceLocation.my_multiline_lambda.source_location.last.should == 27 end it "returns the location of the proc's body; not necessarily the proc itself" do - ProcSpecs::SourceLocation.my_detached_proc.source_location[1].should == 41 - ProcSpecs::SourceLocation.my_detached_proc_new.source_location[1].should == 51 - ProcSpecs::SourceLocation.my_detached_lambda.source_location[1].should == 46 + ProcSpecs::SourceLocation.my_detached_proc.source_location.last.should == 41 + ProcSpecs::SourceLocation.my_detached_proc_new.source_location.last.should == 51 + ProcSpecs::SourceLocation.my_detached_lambda.source_location.last.should == 46 end it "returns the same value for a proc-ified method as the method reports" do @@ -93,12 +86,6 @@ it "works for eval with a given line" do proc = eval('-> {}', nil, "foo", 100) - location = proc.source_location - ruby_version_is(""..."4.0") do - location.should == ["foo", 100] - end - ruby_version_is("4.0") do - location.should == ["foo", 100, 2, 100, 5] - end + proc.source_location.should == ["foo", 100] end end diff --git a/spec/ruby/core/unboundmethod/source_location_spec.rb b/spec/ruby/core/unboundmethod/source_location_spec.rb index 85078ff34e8cd5..5c2f14362c40b4 100644 --- a/spec/ruby/core/unboundmethod/source_location_spec.rb +++ b/spec/ruby/core/unboundmethod/source_location_spec.rb @@ -7,23 +7,23 @@ end it "sets the first value to the path of the file in which the method was defined" do - file = @method.source_location[0] + file = @method.source_location.first file.should be_an_instance_of(String) file.should == File.realpath('fixtures/classes.rb', __dir__) end - it "sets the second value to an Integer representing the line on which the method was defined" do - line = @method.source_location[1] + it "sets the last value to an Integer representing the line on which the method was defined" do + line = @method.source_location.last line.should be_an_instance_of(Integer) line.should == 5 end it "returns the last place the method was defined" do - UnboundMethodSpecs::SourceLocation.method(:redefined).unbind.source_location[1].should == 13 + UnboundMethodSpecs::SourceLocation.method(:redefined).unbind.source_location.last.should == 13 end it "returns the location of the original method even if it was aliased" do - UnboundMethodSpecs::SourceLocation.instance_method(:aka).source_location[1].should == 17 + UnboundMethodSpecs::SourceLocation.instance_method(:aka).source_location.last.should == 17 end it "works for define_method methods" do @@ -54,12 +54,6 @@ c = Class.new do eval('def m; end', nil, "foo", 100) end - location = c.instance_method(:m).source_location - ruby_version_is(""..."4.0") do - location.should == ["foo", 100] - end - ruby_version_is("4.0") do - location.should == ["foo", 100, 0, 100, 10] - end + c.instance_method(:m).source_location.should == ["foo", 100] end end diff --git a/test/ruby/test_lambda.rb b/test/ruby/test_lambda.rb index 3cbb54306c8afa..7738034240497d 100644 --- a/test/ruby/test_lambda.rb +++ b/test/ruby/test_lambda.rb @@ -276,27 +276,27 @@ def test_break end def test_do_lambda_source_location - exp = [__LINE__ + 1, 12, __LINE__ + 5, 7] + exp_lineno = __LINE__ + 3 lmd = ->(x, y, z) do # end - file, *loc = lmd.source_location + file, lineno = lmd.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp, loc) + assert_equal(exp_lineno, lineno, "must be at the beginning of the block") end def test_brace_lambda_source_location - exp = [__LINE__ + 1, 12, __LINE__ + 5, 5] + exp_lineno = __LINE__ + 3 lmd = ->(x, y, z) { # } - file, *loc = lmd.source_location + file, lineno = lmd.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp, loc) + assert_equal(exp_lineno, lineno, "must be at the beginning of the block") end def test_not_orphan_return diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index 92cdfc6757da22..b50875e7b0aa24 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -513,7 +513,7 @@ def test_binding_source_location file, lineno = method(:source_location_test).to_proc.binding.source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(@@line_of_source_location_test[0], lineno, 'Bug #2427') + assert_equal(@@line_of_source_location_test, lineno, 'Bug #2427') end def test_binding_error_unless_ruby_frame @@ -1499,19 +1499,15 @@ def test_to_s assert_include(EnvUtil.labeled_class(name, Proc).new {}.to_s, name) end - @@line_of_source_location_test = [__LINE__ + 1, 2, __LINE__ + 3, 5] + @@line_of_source_location_test = __LINE__ + 1 def source_location_test a=1, b=2 end def test_source_location - file, *loc = method(:source_location_test).source_location + file, lineno = method(:source_location_test).source_location assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(@@line_of_source_location_test, loc, 'Bug #2427') - - file, *loc = self.class.instance_method(:source_location_test).source_location - assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(@@line_of_source_location_test, loc, 'Bug #2427') + assert_equal(@@line_of_source_location_test, lineno, 'Bug #2427') end @@line_of_attr_reader_source_location_test = __LINE__ + 3 @@ -1544,13 +1540,13 @@ def block_source_location_test(*args, &block) end def test_block_source_location - exp_loc = [__LINE__ + 3, 49, __LINE__ + 4, 49] - file, *loc = block_source_location_test(1, + exp_lineno = __LINE__ + 3 + file, lineno = block_source_location_test(1, 2, 3) do end assert_match(/^#{ Regexp.quote(__FILE__) }$/, file) - assert_equal(exp_loc, loc) + assert_equal(exp_lineno, lineno) end def test_splat_without_respond_to From f4c48505ca50d021c3505fe422c04e4f41624bb2 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 15 Dec 2025 21:11:33 +0900 Subject: [PATCH 2/9] Box: move extensions from namespace to box --- ext/-test-/box/yay1/extconf.rb | 2 +- ext/-test-/box/yay2/extconf.rb | 2 +- test/-ext-/box/test_load_ext.rb | 32 ++++++++++++++++---------------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ext/-test-/box/yay1/extconf.rb b/ext/-test-/box/yay1/extconf.rb index 539e99ab091b34..54387cedf13d43 100644 --- a/ext/-test-/box/yay1/extconf.rb +++ b/ext/-test-/box/yay1/extconf.rb @@ -1 +1 @@ -create_makefile('-test-/namespace/yay1') +create_makefile('-test-/box/yay1') diff --git a/ext/-test-/box/yay2/extconf.rb b/ext/-test-/box/yay2/extconf.rb index 2027a42860ba6a..850ef3edc942a7 100644 --- a/ext/-test-/box/yay2/extconf.rb +++ b/ext/-test-/box/yay2/extconf.rb @@ -1 +1 @@ -create_makefile('-test-/namespace/yay2') +create_makefile('-test-/box/yay2') diff --git a/test/-ext-/box/test_load_ext.rb b/test/-ext-/box/test_load_ext.rb index da7fe22a74f72c..cc69e07e0b757e 100644 --- a/test/-ext-/box/test_load_ext.rb +++ b/test/-ext-/box/test_load_ext.rb @@ -8,7 +8,7 @@ def test_load_extension pend assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}") begin; - require '-test-/namespace/yay1' + require '-test-/box/yay1' assert_equal "1.0.0", Yay.version assert_equal "yay", Yay.yay end; @@ -18,24 +18,24 @@ def test_extension_contamination_in_global pend assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; - require '-test-/namespace/yay1' + require '-test-/box/yay1' yay1 = Yay assert_equal "1.0.0", Yay.version assert_equal "yay", Yay.yay - require '-test-/namespace/yay2' + require '-test-/box/yay2' assert_equal "2.0.0", Yay.version v = Yay.yay assert(v == "yay" || v == "yaaay") # "yay" on Linux, "yaaay" on macOS, Win32 end; end - def test_load_extension_in_namespace + def test_load_extension_in_box pend assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") begin; - ns = Namespace.new - ns.require '-test-/namespace/yay1' + ns = Ruby::Box.new + ns.require '-test-/box/yay1' assert_equal "1.0.0", ns::Yay.version assert_raise(NameError) { Yay } end; @@ -45,10 +45,10 @@ def test_different_version_extensions pend assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") begin; - ns1 = Namespace.new - ns2 = Namespace.new - ns1.require('-test-/namespace/yay1') - ns2.require('-test-/namespace/yay2') + ns1 = Ruby::Box.new + ns2 = Ruby::Box.new + ns1.require('-test-/box/yay1') + ns2.require('-test-/box/yay2') assert_raise(NameError) { Yay } assert_not_nil ns1::Yay @@ -64,12 +64,12 @@ def test_loading_extensions_from_global_to_local pend assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") begin; - require '-test-/namespace/yay1' + require '-test-/box/yay1' assert_equal "1.0.0", Yay.version assert_equal "yay", Yay.yay - ns = Namespace.new - ns.require '-test-/namespace/yay2' + ns = Ruby::Box.new + ns.require '-test-/box/yay2' assert_equal "2.0.0", ns::Yay.version assert_equal "yaaay", ns::Yay.yay @@ -81,13 +81,13 @@ def test_loading_extensions_from_local_to_global pend assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") begin; - ns = Namespace.new - ns.require '-test-/namespace/yay1' + ns = Ruby::Box.new + ns.require '-test-/box/yay1' assert_equal "1.0.0", ns::Yay.version assert_equal "yay", ns::Yay.yay - require '-test-/namespace/yay2' + require '-test-/box/yay2' assert_equal "2.0.0", Yay.version assert_equal "yaaay", Yay.yay From 7780d3b6c303d00bc39d913dc91ff9f5f61ae9ed Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 15 Dec 2025 21:12:07 +0900 Subject: [PATCH 3/9] Box: fix the environment variable name --- test/-ext-/box/test_load_ext.rb | 10 +++++----- test/ruby/test_box.rb | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/-ext-/box/test_load_ext.rb b/test/-ext-/box/test_load_ext.rb index cc69e07e0b757e..ea3744375ea38f 100644 --- a/test/-ext-/box/test_load_ext.rb +++ b/test/-ext-/box/test_load_ext.rb @@ -2,7 +2,7 @@ require 'test/unit' class Test_Load_Extensions < Test::Unit::TestCase - ENV_ENABLE_NAMESPACE = {'RUBY_NAMESPACE' => '1'} + ENV_ENABLE_BOX = {'RUBY_BOX' => '1'} def test_load_extension pend @@ -32,7 +32,7 @@ def test_extension_contamination_in_global def test_load_extension_in_box pend - assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") + assert_separately([ENV_ENABLE_BOX], "#{<<~"begin;"}\n#{<<~'end;'}") begin; ns = Ruby::Box.new ns.require '-test-/box/yay1' @@ -43,7 +43,7 @@ def test_load_extension_in_box def test_different_version_extensions pend - assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") + assert_separately([ENV_ENABLE_BOX], "#{<<~"begin;"}\n#{<<~'end;'}") begin; ns1 = Ruby::Box.new ns2 = Ruby::Box.new @@ -62,7 +62,7 @@ def test_different_version_extensions def test_loading_extensions_from_global_to_local pend - assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") + assert_separately([ENV_ENABLE_BOX], "#{<<~"begin;"}\n#{<<~'end;'}") begin; require '-test-/box/yay1' assert_equal "1.0.0", Yay.version @@ -79,7 +79,7 @@ def test_loading_extensions_from_global_to_local def test_loading_extensions_from_local_to_global pend - assert_separately([ENV_ENABLE_NAMESPACE], "#{<<~"begin;"}\n#{<<~'end;'}") + assert_separately([ENV_ENABLE_BOX], "#{<<~"begin;"}\n#{<<~'end;'}") begin; ns = Ruby::Box.new ns.require '-test-/box/yay1' diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index 5d06b60cd77005..4f631254817d28 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -190,7 +190,7 @@ def test_proc_defined_in_box_refers_module_in_box pend unless Ruby::Box.enabled? # require_relative dosn't work well in assert_separately even with __FILE__ and __LINE__ - assert_separately([ENV_ENABLE_NAMESPACE], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; ns1 = Namespace.new ns1.require(File.join("#{here}", 'namespace/proc_callee')) @@ -212,7 +212,7 @@ def test_proc_defined_globally_refers_global_module pend unless Ruby::Box.enabled? # require_relative dosn't work well in assert_separately even with __FILE__ and __LINE__ - assert_separately([ENV_ENABLE_NAMESPACE], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; require(File.join("#{here}", 'namespace/proc_callee')) def Target.foo From 8db3642ab54f303b32a02bbfe53a2ab54725b046 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 15 Dec 2025 21:13:11 +0900 Subject: [PATCH 4/9] Box: fix the class name in inspect --- box.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/box.c b/box.c index 150a75700c9313..77a89de317d298 100644 --- a/box.c +++ b/box.c @@ -929,11 +929,11 @@ rb_box_inspect(VALUE obj) rb_box_t *box; VALUE r; if (obj == Qfalse) { - r = rb_str_new_cstr("#"); + r = rb_str_new_cstr("#"); return r; } box = rb_get_box_t(obj); - r = rb_str_new_cstr("#box_id), rb_intern("to_s"), 0)); if (BOX_ROOT_P(box)) { rb_str_cat_cstr(r, ",root"); From 5f09e1f046599c5466787449f5d30330d4dd4aab Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 15 Dec 2025 21:28:17 +0900 Subject: [PATCH 5/9] Box: [DOC] fix the class name in rdoc Also remove a stale TODO. --- box.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/box.c b/box.c index 77a89de317d298..14f6acdd8267b5 100644 --- a/box.c +++ b/box.c @@ -348,7 +348,7 @@ rb_get_box_object(rb_box_t *box) /* * call-seq: - * Namespace.new -> new_box + * Ruby::Box.new -> new_box * * Returns a new Ruby::Box object. */ @@ -389,7 +389,7 @@ box_initialize(VALUE box_value) /* * call-seq: - * Namespace.enabled? -> true or false + * Ruby::Box.enabled? -> true or false * * Returns +true+ if Ruby::Box is enabled. */ @@ -401,7 +401,7 @@ rb_box_s_getenabled(VALUE recv) /* * call-seq: - * Namespace.current -> box, nil or false + * Ruby::Box.current -> box, nil or false * * Returns the current box. * Returns +nil+ if Ruby Box is not enabled. @@ -795,10 +795,6 @@ rb_box_local_extension(VALUE box_value, VALUE fname, VALUE path, VALUE *cleanup) return new_path; } -// TODO: delete it just after dln_load? or delay it? -// At least for _WIN32, deleting extension files should be delayed until the namespace's destructor. -// And it requires calling dlclose before deleting it. - static VALUE rb_box_load(int argc, VALUE *argv, VALUE box) { From 85b40c5ea8f606cf34cab8a5b1277033bede2457 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 16 Dec 2025 09:01:14 +0900 Subject: [PATCH 6/9] Box: fix the class name in tests --- spec/ruby/core/module/ancestors_spec.rb | 6 +- test/ruby/box/a.1_1_0.rb | 4 +- test/ruby/box/a.1_2_0.rb | 4 +- test/ruby/box/a.rb | 4 +- test/ruby/box/autoloading.rb | 6 +- test/ruby/box/box.rb | 10 + test/ruby/box/consts.rb | 1 + test/ruby/box/current.rb | 13 - test/ruby/box/global_vars.rb | 8 +- test/ruby/box/ns.rb | 10 - test/ruby/box/procs.rb | 2 +- test/ruby/test_box.rb | 578 ++++++++++++------------ 12 files changed, 321 insertions(+), 325 deletions(-) create mode 100644 test/ruby/box/box.rb delete mode 100644 test/ruby/box/current.rb delete mode 100644 test/ruby/box/ns.rb diff --git a/spec/ruby/core/module/ancestors_spec.rb b/spec/ruby/core/module/ancestors_spec.rb index 34679575b5dd95..90c26941d14a1b 100644 --- a/spec/ruby/core/module/ancestors_spec.rb +++ b/spec/ruby/core/module/ancestors_spec.rb @@ -7,11 +7,11 @@ ModuleSpecs.ancestors.should == [ModuleSpecs] ModuleSpecs::Basic.ancestors.should == [ModuleSpecs::Basic] ModuleSpecs::Super.ancestors.should == [ModuleSpecs::Super, ModuleSpecs::Basic] - if defined?(Namespace) && Namespace.enabled? + if defined?(Ruby::Box) && Ruby::Box.enabled? ModuleSpecs.without_test_modules(ModuleSpecs::Parent.ancestors).should == - [ModuleSpecs::Parent, Object, Namespace::Loader, Kernel, BasicObject] + [ModuleSpecs::Parent, Object, Ruby::Box::Loader, Kernel, BasicObject] ModuleSpecs.without_test_modules(ModuleSpecs::Child.ancestors).should == - [ModuleSpecs::Child, ModuleSpecs::Super, ModuleSpecs::Basic, ModuleSpecs::Parent, Object, Namespace::Loader, Kernel, BasicObject] + [ModuleSpecs::Child, ModuleSpecs::Super, ModuleSpecs::Basic, ModuleSpecs::Parent, Object, Ruby::Box::Loader, Kernel, BasicObject] else ModuleSpecs.without_test_modules(ModuleSpecs::Parent.ancestors).should == [ModuleSpecs::Parent, Object, Kernel, BasicObject] diff --git a/test/ruby/box/a.1_1_0.rb b/test/ruby/box/a.1_1_0.rb index bf64dbaa62264c..032258509759be 100644 --- a/test/ruby/box/a.1_1_0.rb +++ b/test/ruby/box/a.1_1_0.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class NS_A +class BOX_A VERSION = "1.1.0" def yay @@ -8,7 +8,7 @@ def yay end end -module NS_B +module BOX_B VERSION = "1.1.0" def self.yay diff --git a/test/ruby/box/a.1_2_0.rb b/test/ruby/box/a.1_2_0.rb index 6d25c0885dc2eb..29813ea57b2a70 100644 --- a/test/ruby/box/a.1_2_0.rb +++ b/test/ruby/box/a.1_2_0.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class NS_A +class BOX_A VERSION = "1.2.0" def yay @@ -8,7 +8,7 @@ def yay end end -module NS_B +module BOX_B VERSION = "1.2.0" def self.yay diff --git a/test/ruby/box/a.rb b/test/ruby/box/a.rb index a6dcd9cd21ce25..26a622c92b5fb2 100644 --- a/test/ruby/box/a.rb +++ b/test/ruby/box/a.rb @@ -1,4 +1,4 @@ -class NS_A +class BOX_A FOO = "foo_a1" def yay @@ -6,7 +6,7 @@ def yay end end -module NS_B +module BOX_B BAR = "bar_b1" def self.yay diff --git a/test/ruby/box/autoloading.rb b/test/ruby/box/autoloading.rb index 19ec00bcd5a15d..cba57ab3776e91 100644 --- a/test/ruby/box/autoloading.rb +++ b/test/ruby/box/autoloading.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -autoload :NS_A, File.join(__dir__, 'a.1_1_0') -NS_A.new.yay +autoload :BOX_A, File.join(__dir__, 'a.1_1_0') +BOX_A.new.yay -module NS_B +module BOX_B autoload :BAR, File.join(__dir__, 'a') end diff --git a/test/ruby/box/box.rb b/test/ruby/box/box.rb new file mode 100644 index 00000000000000..3b7da14e9d7ef5 --- /dev/null +++ b/test/ruby/box/box.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +BOX1 = Ruby::Box.new +BOX1.require_relative('a.1_1_0') + +def yay + BOX1::BOX_B::yay +end + +yay diff --git a/test/ruby/box/consts.rb b/test/ruby/box/consts.rb index 44a383111b7685..e40cd5c50c7fe5 100644 --- a/test/ruby/box/consts.rb +++ b/test/ruby/box/consts.rb @@ -1,3 +1,4 @@ +$VERBOSE = nil class String STR_CONST1 = 111 STR_CONST2 = 222 diff --git a/test/ruby/box/current.rb b/test/ruby/box/current.rb deleted file mode 100644 index 4af9a92ddc5d5b..00000000000000 --- a/test/ruby/box/current.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -$ns_in_ns = ::Namespace.current - -module CurrentNamespace - def self.in_require - $ns_in_ns - end - - def self.in_method_call - ::Namespace.current - end -end diff --git a/test/ruby/box/global_vars.rb b/test/ruby/box/global_vars.rb index 3764eb0d19efc2..590363f6173fe8 100644 --- a/test/ruby/box/global_vars.rb +++ b/test/ruby/box/global_vars.rb @@ -20,18 +20,18 @@ def self.write(char) module UniqueGvar def self.read - $used_only_in_ns + $used_only_in_box end def self.write(val) - $used_only_in_ns = val + $used_only_in_box = val end def self.write_only(val) - $write_only_var_in_ns = val + $write_only_var_in_box = val end - def self.gvars_in_ns + def self.gvars_in_box global_variables end end diff --git a/test/ruby/box/ns.rb b/test/ruby/box/ns.rb deleted file mode 100644 index e947e3cdc830aa..00000000000000 --- a/test/ruby/box/ns.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -NS1 = Namespace.new -NS1.require_relative('a.1_1_0') - -def yay - NS1::NS_B::yay -end - -yay diff --git a/test/ruby/box/procs.rb b/test/ruby/box/procs.rb index a7fe58ceb6ecc6..1c39a8231bf463 100644 --- a/test/ruby/box/procs.rb +++ b/test/ruby/box/procs.rb @@ -11,7 +11,7 @@ module B end end -module ProcInNS +module ProcInBox def self.make_proc_from_block(&b) b end diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index 4f631254817d28..b62ac3c544722d 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -10,16 +10,21 @@ class TestBox < Test::Unit::TestCase ENV_ENABLE_BOX = {'RUBY_BOX' => '1', 'TEST_DIR' => __dir__} def setup - @n = nil + @box = nil @dir = __dir__ end def teardown - @n = nil + @box = nil + end + + def setup_box + pend unless Ruby::Box.enabled? + @box = Ruby::Box.new end def test_box_availability_in_default - assert_separately([], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + assert_separately(['RUBY_BOX'=>nil], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; assert_nil ENV['RUBY_BOX'] assert !Ruby::Box.enabled? @@ -43,278 +48,278 @@ def test_current_box_in_main end def test_require_rb_separately - pend unless Ruby::Box.enabled? + setup_box - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } - @n.require(File.join(__dir__, 'namespace', 'a.1_1_0')) + @box.require(File.join(__dir__, 'box', 'a.1_1_0')) - assert_not_nil @n::NS_A - assert_not_nil @n::NS_B - assert_equal "1.1.0", @n::NS_A::VERSION - assert_equal "yay 1.1.0", @n::NS_A.new.yay - assert_equal "1.1.0", @n::NS_B::VERSION - assert_equal "yay_b1", @n::NS_B.yay + assert_not_nil @box::BOX_A + assert_not_nil @box::BOX_B + assert_equal "1.1.0", @box::BOX_A::VERSION + assert_equal "yay 1.1.0", @box::BOX_A.new.yay + assert_equal "1.1.0", @box::BOX_B::VERSION + assert_equal "yay_b1", @box::BOX_B.yay - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } end def test_require_relative_rb_separately - pend unless Ruby::Box.enabled? + setup_box - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } - @n.require_relative('namespace/a.1_1_0') + @box.require_relative('box/a.1_1_0') - assert_not_nil @n::NS_A - assert_not_nil @n::NS_B - assert_equal "1.1.0", @n::NS_A::VERSION - assert_equal "yay 1.1.0", @n::NS_A.new.yay - assert_equal "1.1.0", @n::NS_B::VERSION - assert_equal "yay_b1", @n::NS_B.yay + assert_not_nil @box::BOX_A + assert_not_nil @box::BOX_B + assert_equal "1.1.0", @box::BOX_A::VERSION + assert_equal "yay 1.1.0", @box::BOX_A.new.yay + assert_equal "1.1.0", @box::BOX_B::VERSION + assert_equal "yay_b1", @box::BOX_B.yay - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } end def test_load_separately - pend unless Ruby::Box.enabled? + setup_box - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } - @n.load(File.join(__dir__, 'namespace', 'a.1_1_0.rb')) + @box.load(File.join(__dir__, 'box', 'a.1_1_0.rb')) - assert_not_nil @n::NS_A - assert_not_nil @n::NS_B - assert_equal "1.1.0", @n::NS_A::VERSION - assert_equal "yay 1.1.0", @n::NS_A.new.yay - assert_equal "1.1.0", @n::NS_B::VERSION - assert_equal "yay_b1", @n::NS_B.yay + assert_not_nil @box::BOX_A + assert_not_nil @box::BOX_B + assert_equal "1.1.0", @box::BOX_A::VERSION + assert_equal "yay 1.1.0", @box::BOX_A.new.yay + assert_equal "1.1.0", @box::BOX_B::VERSION + assert_equal "yay_b1", @box::BOX_B.yay - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } end def test_box_in_box - pend unless Ruby::Box.enabled? + setup_box - assert_raise(NameError) { NS1 } - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX1 } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } - @n.require_relative('namespace/ns') + @box.require_relative('box/box') - assert_not_nil @n::NS1 - assert_not_nil @n::NS1::NS_A - assert_not_nil @n::NS1::NS_B - assert_equal "1.1.0", @n::NS1::NS_A::VERSION - assert_equal "yay 1.1.0", @n::NS1::NS_A.new.yay - assert_equal "1.1.0", @n::NS1::NS_B::VERSION - assert_equal "yay_b1", @n::NS1::NS_B.yay + assert_not_nil @box::BOX1 + assert_not_nil @box::BOX1::BOX_A + assert_not_nil @box::BOX1::BOX_B + assert_equal "1.1.0", @box::BOX1::BOX_A::VERSION + assert_equal "yay 1.1.0", @box::BOX1::BOX_A.new.yay + assert_equal "1.1.0", @box::BOX1::BOX_B::VERSION + assert_equal "yay_b1", @box::BOX1::BOX_B.yay - assert_raise(NameError) { NS1 } - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX1 } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } end - def test_require_rb_2versions - pend unless Ruby::Box.enabled? + def test_require_rb_2versiobox + setup_box - assert_raise(NameError) { NS_A } + assert_raise(NameError) { BOX_A } - @n.require(File.join(__dir__, 'namespace', 'a.1_2_0')) - assert_equal "1.2.0", @n::NS_A::VERSION - assert_equal "yay 1.2.0", @n::NS_A.new.yay + @box.require(File.join(__dir__, 'box', 'a.1_2_0')) + assert_equal "1.2.0", @box::BOX_A::VERSION + assert_equal "yay 1.2.0", @box::BOX_A.new.yay - n2 = Namespace.new - n2.require(File.join(__dir__, 'namespace', 'a.1_1_0')) - assert_equal "1.1.0", n2::NS_A::VERSION - assert_equal "yay 1.1.0", n2::NS_A.new.yay + n2 = Ruby::Box.new + n2.require(File.join(__dir__, 'box', 'a.1_1_0')) + assert_equal "1.1.0", n2::BOX_A::VERSION + assert_equal "yay 1.1.0", n2::BOX_A.new.yay - # recheck @n is not affected by the following require - assert_equal "1.2.0", @n::NS_A::VERSION - assert_equal "yay 1.2.0", @n::NS_A.new.yay + # recheck @box is not affected by the following require + assert_equal "1.2.0", @box::BOX_A::VERSION + assert_equal "yay 1.2.0", @box::BOX_A.new.yay - assert_raise(NameError) { NS_A } + assert_raise(NameError) { BOX_A } end def test_raising_errors_in_require - pend unless Ruby::Box.enabled? + setup_box - assert_raise(RuntimeError, "Yay!") { @n.require(File.join(__dir__, 'namespace', 'raise')) } - assert Namespace.current.inspect.include?("main") + assert_raise(RuntimeError, "Yay!") { @box.require(File.join(__dir__, 'box', 'raise')) } + assert Ruby::Box.current.inspect.include?("main") end def test_autoload_in_box - pend unless Ruby::Box.enabled? + setup_box - assert_raise(NameError) { NS_A } + assert_raise(NameError) { BOX_A } - @n.require_relative('namespace/autoloading') + @box.require_relative('box/autoloading') # autoloaded A is visible from global - assert_equal '1.1.0', @n::NS_A::VERSION + assert_equal '1.1.0', @box::BOX_A::VERSION - assert_raise(NameError) { NS_A } + assert_raise(NameError) { BOX_A } - # autoload trigger NS_B::BAR is valid even from global - assert_equal 'bar_b1', @n::NS_B::BAR + # autoload trigger BOX_B::BAR is valid even from global + assert_equal 'bar_b1', @box::BOX_B::BAR - assert_raise(NameError) { NS_A } - assert_raise(NameError) { NS_B } + assert_raise(NameError) { BOX_A } + assert_raise(NameError) { BOX_B } end def test_continuous_top_level_method_in_a_box - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/define_toplevel') - @n.require_relative('namespace/call_toplevel') + @box.require_relative('box/define_toplevel') + @box.require_relative('box/call_toplevel') assert_raise(NameError) { foo } end def test_top_level_methods_in_box pend # TODO: fix loading/current box detection - pend unless Ruby::Box.enabled? - @n.require_relative('box/top_level') - assert_equal "yay!", @n::Foo.foo + setup_box + @box.require_relative('box/top_level') + assert_equal "yay!", @box::Foo.foo assert_raise(NameError) { yaaay } - assert_equal "foo", @n::Bar.bar - assert_raise_with_message(RuntimeError, "boooo") { @n::Baz.baz } + assert_equal "foo", @box::Bar.bar + assert_raise_with_message(RuntimeError, "boooo") { @box::Baz.baz } end def test_proc_defined_in_box_refers_module_in_box - pend unless Ruby::Box.enabled? + setup_box # require_relative dosn't work well in assert_separately even with __FILE__ and __LINE__ assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; - ns1 = Namespace.new - ns1.require(File.join("#{here}", 'namespace/proc_callee')) - proc_v = ns1::Foo.callee + box1 = Ruby::Box.new + box1.require("#{here}/box/proc_callee") + proc_v = box1::Foo.callee assert_raise(NameError) { Target } - assert ns1::Target - assert_equal "fooooo", proc_v.call # refers Target in the namespace ns1 - ns1.require(File.join("#{here}", 'namespace/proc_caller')) - assert_equal "fooooo", ns1::Bar.caller(proc_v) - - ns2 = Namespace.new - ns2.require(File.join("#{here}", 'namespace/proc_caller')) - assert_raise(NameError) { ns2::Target } - assert_equal "fooooo", ns2::Bar.caller(proc_v) # refers Target in the namespace ns1 + assert box1::Target + assert_equal "fooooo", proc_v.call # refers Target in the box box1 + box1.require("#{here}/box/proc_caller") + assert_equal "fooooo", box1::Bar.caller(proc_v) + + box2 = Ruby::Box.new + box2.require("#{here}/box/proc_caller") + assert_raise(NameError) { box2::Target } + assert_equal "fooooo", box2::Bar.caller(proc_v) # refers Target in the box box1 end; end def test_proc_defined_globally_refers_global_module - pend unless Ruby::Box.enabled? + setup_box # require_relative dosn't work well in assert_separately even with __FILE__ and __LINE__ assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "here = '#{__dir__}'; #{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; - require(File.join("#{here}", 'namespace/proc_callee')) + require("#{here}/box/proc_callee") def Target.foo "yay" end proc_v = Foo.callee assert Target assert_equal "yay", proc_v.call # refers global Foo - ns1 = Namespace.new - ns1.require(File.join("#{here}", 'namespace/proc_caller')) - assert_equal "yay", ns1::Bar.caller(proc_v) - - ns2 = Namespace.new - ns2.require(File.join("#{here}", 'namespace/proc_callee')) - ns2.require(File.join("#{here}", 'namespace/proc_caller')) - assert_equal "fooooo", ns2::Foo.callee.call - assert_equal "yay", ns2::Bar.caller(proc_v) # should refer the global Target, not Foo in ns2 + box1 = Ruby::Box.new + box1.require("#{here}/box/proc_caller") + assert_equal "yay", box1::Bar.caller(proc_v) + + box2 = Ruby::Box.new + box2.require("#{here}/box/proc_callee") + box2.require("#{here}/box/proc_caller") + assert_equal "fooooo", box2::Foo.callee.call + assert_equal "yay", box2::Bar.caller(proc_v) # should refer the global Target, not Foo in box2 end; end def test_instance_variable - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/instance_variables') + @box.require_relative('box/instance_variables') assert_equal [], String.instance_variables - assert_equal [:@str_ivar1, :@str_ivar2], @n::StringDelegatorObj.instance_variables - assert_equal 111, @n::StringDelegatorObj.str_ivar1 - assert_equal 222, @n::StringDelegatorObj.str_ivar2 - assert_equal 222, @n::StringDelegatorObj.instance_variable_get(:@str_ivar2) + assert_equal [:@str_ivar1, :@str_ivar2], @box::StringDelegatorObj.instance_variables + assert_equal 111, @box::StringDelegatorObj.str_ivar1 + assert_equal 222, @box::StringDelegatorObj.str_ivar2 + assert_equal 222, @box::StringDelegatorObj.instance_variable_get(:@str_ivar2) - @n::StringDelegatorObj.instance_variable_set(:@str_ivar3, 333) - assert_equal 333, @n::StringDelegatorObj.instance_variable_get(:@str_ivar3) - @n::StringDelegatorObj.remove_instance_variable(:@str_ivar1) - assert_nil @n::StringDelegatorObj.str_ivar1 - assert_equal [:@str_ivar2, :@str_ivar3], @n::StringDelegatorObj.instance_variables + @box::StringDelegatorObj.instance_variable_set(:@str_ivar3, 333) + assert_equal 333, @box::StringDelegatorObj.instance_variable_get(:@str_ivar3) + @box::StringDelegatorObj.remove_instance_variable(:@str_ivar1) + assert_nil @box::StringDelegatorObj.str_ivar1 + assert_equal [:@str_ivar2, :@str_ivar3], @box::StringDelegatorObj.instance_variables assert_equal [], String.instance_variables end def test_methods_added_in_box_are_invisible_globally - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/string_ext') + @box.require_relative('box/string_ext') - assert_equal "yay", @n::Bar.yay + assert_equal "yay", @box::Bar.yay assert_raise(NoMethodError){ String.new.yay } end def test_continuous_method_definitions_in_a_box - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/string_ext') - assert_equal "yay", @n::Bar.yay + @box.require_relative('box/string_ext') + assert_equal "yay", @box::Bar.yay - @n.require_relative('namespace/string_ext_caller') - assert_equal "yay", @n::Foo.yay + @box.require_relative('box/string_ext_caller') + assert_equal "yay", @box::Foo.yay - @n.require_relative('namespace/string_ext_calling') + @box.require_relative('box/string_ext_calling') end def test_methods_added_in_box_later_than_caller_code - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/string_ext_caller') - @n.require_relative('namespace/string_ext') + @box.require_relative('box/string_ext_caller') + @box.require_relative('box/string_ext') - assert_equal "yay", @n::Bar.yay - assert_equal "yay", @n::Foo.yay + assert_equal "yay", @box::Bar.yay + assert_equal "yay", @box::Foo.yay end def test_method_added_in_box_are_available_on_eval - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/string_ext') - @n.require_relative('namespace/string_ext_eval_caller') + @box.require_relative('box/string_ext') + @box.require_relative('box/string_ext_eval_caller') - assert_equal "yay", @n::Baz.yay + assert_equal "yay", @box::Baz.yay end def test_method_added_in_box_are_available_on_eval_with_binding - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/string_ext') - @n.require_relative('namespace/string_ext_eval_caller') + @box.require_relative('box/string_ext') + @box.require_relative('box/string_ext_eval_caller') - assert_equal "yay, yay!", @n::Baz.yay_with_binding + assert_equal "yay, yay!", @box::Baz.yay_with_binding end def test_methods_and_constants_added_by_include - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/open_class_with_include') + @box.require_relative('box/open_class_with_include') - assert_equal "I'm saying foo 1", @n::OpenClassWithInclude.say - assert_equal "I'm saying foo 1", @n::OpenClassWithInclude.say_foo - assert_equal "I'm saying foo 1", @n::OpenClassWithInclude.say_with_obj("wow") + assert_equal "I'm saying foo 1", @box::OpenClassWithInclude.say + assert_equal "I'm saying foo 1", @box::OpenClassWithInclude.say_foo + assert_equal "I'm saying foo 1", @box::OpenClassWithInclude.say_with_obj("wow") assert_raise(NameError) { String::FOO } - assert_equal "foo 1", @n::OpenClassWithInclude.refer_foo + assert_equal "foo 1", @box::OpenClassWithInclude.refer_foo end end @@ -330,9 +335,9 @@ def make_proc_from_block(&b) end def test_proc_from_main_works_with_global_definitions - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/procs') + @box.require_relative('box/procs') proc_and_labels = [ [Proc.new { String.new.yay }, "Proc.new"], @@ -340,13 +345,13 @@ def test_proc_from_main_works_with_global_definitions [lambda { String.new.yay }, "lambda{}"], [->(){ String.new.yay }, "->(){}"], [make_proc_from_block { String.new.yay }, "make_proc_from_block"], - [@n::ProcInNS.make_proc_from_block { String.new.yay }, "make_proc_from_block in @n"], + [@box::ProcInBox.make_proc_from_block { String.new.yay }, "make_proc_from_block in @box"], ] proc_and_labels.each do |str_pr| pr, pr_label = str_pr assert_raise(NoMethodError, "NoMethodError expected: #{pr_label}, called in main") { pr.call } - assert_raise(NoMethodError, "NoMethodError expected: #{pr_label}, called in @n") { @n::ProcInNS.call_proc(pr) } + assert_raise(NoMethodError, "NoMethodError expected: #{pr_label}, called in @box") { @box::ProcInBox.call_proc(pr) } end const_and_labels = [ @@ -355,48 +360,48 @@ def test_proc_from_main_works_with_global_definitions [lambda { ProcLookupTestA::B::VALUE }, "lambda{}"], [->(){ ProcLookupTestA::B::VALUE }, "->(){}"], [make_proc_from_block { ProcLookupTestA::B::VALUE }, "make_proc_from_block"], - [@n::ProcInNS.make_proc_from_block { ProcLookupTestA::B::VALUE }, "make_proc_from_block in @n"], + [@box::ProcInBox.make_proc_from_block { ProcLookupTestA::B::VALUE }, "make_proc_from_block in @box"], ] const_and_labels.each do |const_pr| pr, pr_label = const_pr assert_equal 111, pr.call, "111 expected, #{pr_label} called in main" - assert_equal 111, @n::ProcInNS.call_proc(pr), "111 expected, #{pr_label} called in @n" + assert_equal 111, @box::ProcInBox.call_proc(pr), "111 expected, #{pr_label} called in @box" end end def test_proc_from_box_works_with_definitions_in_box - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/procs') + @box.require_relative('box/procs') proc_types = [:proc_new, :proc_f, :lambda_f, :lambda_l, :block] proc_types.each do |proc_type| - assert_equal 222, @n::ProcInNS.make_const_proc(proc_type).call, "ProcLookupTestA::B::VALUE should be 222 in @n" - assert_equal "foo", @n::ProcInNS.make_str_const_proc(proc_type).call, "String::FOO should be \"foo\" in @n" - assert_equal "yay", @n::ProcInNS.make_str_proc(proc_type).call, "String#yay should be callable in @n" + assert_equal 222, @box::ProcInBox.make_const_proc(proc_type).call, "ProcLookupTestA::B::VALUE should be 222 in @box" + assert_equal "foo", @box::ProcInBox.make_str_const_proc(proc_type).call, "String::FOO should be \"foo\" in @box" + assert_equal "yay", @box::ProcInBox.make_str_proc(proc_type).call, "String#yay should be callable in @box" # - # TODO: method calls not-in-methods nor procs can't handle the current namespace correctly. + # TODO: method calls not-in-methods nor procs can't handle the current box correctly. # # assert_equal "yay,foo,222", - # @n::ProcInNS.const_get(('CONST_' + proc_type.to_s.upcase).to_sym).call, - # "Proc assigned to constants should refer constants correctly in @n" + # @box::ProcInBox.const_get(('CONST_' + proc_type.to_s.upcase).to_sym).call, + # "Proc assigned to constants should refer constants correctly in @box" end end def test_class_module_singleton_methods - pend unless Ruby::Box.enabled? + setup_box - @n.require_relative('namespace/singleton_methods') + @box.require_relative('box/singleton_methods') - assert_equal "Good evening!", @n::SingletonMethods.string_greeing # def self.greeting - assert_equal 42, @n::SingletonMethods.integer_answer # class << self; def answer - assert_equal([], @n::SingletonMethods.array_blank) # def self.blank w/ instance methods - assert_equal({status: 200, body: 'OK'}, @n::SingletonMethods.hash_http_200) # class << self; def ... w/ instance methods + assert_equal "Good evening!", @box::SingletonMethods.string_greeing # def self.greeting + assert_equal 42, @box::SingletonMethods.integer_answer # class << self; def answer + assert_equal([], @box::SingletonMethods.array_blank) # def self.blank w/ instance methods + assert_equal({status: 200, body: 'OK'}, @box::SingletonMethods.hash_http_200) # class << self; def ... w/ instance methods - assert_equal([4, 4], @n::SingletonMethods.array_instance_methods_return_size([1, 2, 3, 4])) - assert_equal([3, 3], @n::SingletonMethods.hash_instance_methods_return_size({a: 2, b: 4, c: 8})) + assert_equal([4, 4], @box::SingletonMethods.array_instance_methods_return_size([1, 2, 3, 4])) + assert_equal([3, 3], @box::SingletonMethods.hash_instance_methods_return_size({a: 2, b: 4, c: 8})) assert_raise(NoMethodError) { String.greeting } assert_raise(NoMethodError) { Integer.answer } @@ -405,7 +410,9 @@ def test_class_module_singleton_methods end def test_add_constants_in_box - pend unless Ruby::Box.enabled? + setup_box + + @box.require('envutil') String.const_set(:STR_CONST0, 999) assert_equal 999, String::STR_CONST0 @@ -416,37 +423,38 @@ def test_add_constants_in_box assert_raise(NameError) { String::STR_CONST3 } assert_raise(NameError) { Integer.const_get(:INT_CONST1) } - EnvUtil.suppress_warning do - @n.require_relative('namespace/consts') + EnvUtil.verbose_warning do + @box.require_relative('box/consts') end + return assert_equal 999, String::STR_CONST0 assert_raise(NameError) { String::STR_CONST1 } assert_raise(NameError) { String::STR_CONST2 } assert_raise(NameError) { Integer::INT_CONST1 } - assert_not_nil @n::ForConsts.refer_all + assert_not_nil @box::ForConsts.refer_all - assert_equal 112, @n::ForConsts.refer1 - assert_equal 112, @n::ForConsts.get1 - assert_equal 112, @n::ForConsts::CONST1 - assert_equal 222, @n::ForConsts.refer2 - assert_equal 222, @n::ForConsts.get2 - assert_equal 222, @n::ForConsts::CONST2 - assert_equal 333, @n::ForConsts.refer3 - assert_equal 333, @n::ForConsts.get3 - assert_equal 333, @n::ForConsts::CONST3 + assert_equal 112, @box::ForConsts.refer1 + assert_equal 112, @box::ForConsts.get1 + assert_equal 112, @box::ForConsts::CONST1 + assert_equal 222, @box::ForConsts.refer2 + assert_equal 222, @box::ForConsts.get2 + assert_equal 222, @box::ForConsts::CONST2 + assert_equal 333, @box::ForConsts.refer3 + assert_equal 333, @box::ForConsts.get3 + assert_equal 333, @box::ForConsts::CONST3 - EnvUtil.suppress_warning do - @n::ForConsts.const_set(:CONST3, 334) + @box::EnvUtil.suppress_warning do + @box::ForConsts.const_set(:CONST3, 334) end - assert_equal 334, @n::ForConsts::CONST3 - assert_equal 334, @n::ForConsts.refer3 - assert_equal 334, @n::ForConsts.get3 + assert_equal 334, @box::ForConsts::CONST3 + assert_equal 334, @box::ForConsts.refer3 + assert_equal 334, @box::ForConsts.get3 - assert_equal 10, @n::ForConsts.refer_top_const + assert_equal 10, @box::ForConsts.refer_top_const # use Proxy object to use usual methods instead of singleton methods - proxy = @n::ForConsts::Proxy.new + proxy = @box::ForConsts::Proxy.new assert_raise(NameError){ proxy.call_str_refer0 } assert_raise(NameError){ proxy.call_str_get0 } @@ -486,36 +494,36 @@ def test_global_variables default_l = $-0 default_f = $, - pend unless Ruby::Box.enabled? + setup_box assert_equal "\n", $-0 # equal to $/, line splitter assert_equal nil, $, # field splitter - @n.require_relative('namespace/global_vars') + @box.require_relative('box/global_vars') # read first - assert_equal "\n", @n::LineSplitter.read - @n::LineSplitter.write("\r\n") - assert_equal "\r\n", @n::LineSplitter.read + assert_equal "\n", @box::LineSplitter.read + @box::LineSplitter.write("\r\n") + assert_equal "\r\n", @box::LineSplitter.read assert_equal "\n", $-0 # write first - @n::FieldSplitter.write(",") - assert_equal ",", @n::FieldSplitter.read + @box::FieldSplitter.write(",") + assert_equal ",", @box::FieldSplitter.read assert_equal nil, $, - # used only in ns - assert !global_variables.include?(:$used_only_in_ns) - @n::UniqueGvar.write(123) - assert_equal 123, @n::UniqueGvar.read - assert_nil $used_only_in_ns + # used only in box + assert !global_variables.include?(:$used_only_in_box) + @box::UniqueGvar.write(123) + assert_equal 123, @box::UniqueGvar.read + assert_nil $used_only_in_box # Kernel#global_variables returns the sum of all gvars. global_gvars = global_variables.sort - assert_equal global_gvars, @n::UniqueGvar.gvars_in_ns.sort - @n::UniqueGvar.write_only(456) - assert_equal (global_gvars + [:$write_only_var_in_ns]).sort, @n::UniqueGvar.gvars_in_ns.sort - assert_equal (global_gvars + [:$write_only_var_in_ns]).sort, global_variables.sort + assert_equal global_gvars, @box::UniqueGvar.gvars_in_box.sort + @box::UniqueGvar.write_only(456) + assert_equal (global_gvars + [:$write_only_var_in_box]).sort, @box::UniqueGvar.gvars_in_box.sort + assert_equal (global_gvars + [:$write_only_var_in_box]).sort, global_variables.sort ensure EnvUtil.suppress_warning do $-0 = default_l @@ -524,105 +532,105 @@ def test_global_variables end def test_load_path_and_loaded_features - pend unless Ruby::Box.enabled? + setup_box assert $LOAD_PATH.respond_to?(:resolve_feature_path) - @n.require_relative('namespace/load_path') + @box.require_relative('box/load_path') - assert_not_equal $LOAD_PATH, @n::LoadPathCheck::FIRST_LOAD_PATH + assert_not_equal $LOAD_PATH, @box::LoadPathCheck::FIRST_LOAD_PATH - assert @n::LoadPathCheck::FIRST_LOAD_PATH_RESPOND_TO_RESOLVE + assert @box::LoadPathCheck::FIRST_LOAD_PATH_RESPOND_TO_RESOLVE - namespace_dir = File.join(__dir__, 'namespace') - # TODO: $LOADED_FEATURES in method calls should refer the current namespace in addition to the loading namespace. - # assert @n::LoadPathCheck.current_loaded_features.include?(File.join(namespace_dir, 'blank1.rb')) - # assert !@n::LoadPathCheck.current_loaded_features.include?(File.join(namespace_dir, 'blank2.rb')) - # assert @n::LoadPathCheck.require_blank2 - # assert @n::LoadPathCheck.current_loaded_features.include?(File.join(namespace_dir, 'blank2.rb')) + box_dir = File.join(__dir__, 'box') + # TODO: $LOADED_FEATURES in method calls should refer the current box in addition to the loading box. + # assert @box::LoadPathCheck.current_loaded_features.include?(File.join(box_dir, 'blank1.rb')) + # assert !@box::LoadPathCheck.current_loaded_features.include?(File.join(box_dir, 'blank2.rb')) + # assert @box::LoadPathCheck.require_blank2 + # assert @box::LoadPathCheck.current_loaded_features.include?(File.join(box_dir, 'blank2.rb')) - assert !$LOADED_FEATURES.include?(File.join(namespace_dir, 'blank1.rb')) - assert !$LOADED_FEATURES.include?(File.join(namespace_dir, 'blank2.rb')) + assert !$LOADED_FEATURES.include?(File.join(box_dir, 'blank1.rb')) + assert !$LOADED_FEATURES.include?(File.join(box_dir, 'blank2.rb')) end def test_eval_basic - pend unless Ruby::Box.enabled? + setup_box # Test basic evaluation - result = @n.eval("1 + 1") + result = @box.eval("1 + 1") assert_equal 2, result # Test string evaluation - result = @n.eval("'hello ' + 'world'") + result = @box.eval("'hello ' + 'world'") assert_equal "hello world", result end def test_eval_with_constants - pend unless Ruby::Box.enabled? + setup_box - # Define a constant in the namespace via eval - @n.eval("TEST_CONST = 42") - assert_equal 42, @n::TEST_CONST + # Define a constant in the box via eval + @box.eval("TEST_CONST = 42") + assert_equal 42, @box::TEST_CONST - # Constant should not be visible in main namespace + # Constant should not be visible in main box assert_raise(NameError) { TEST_CONST } end def test_eval_with_classes - pend unless Ruby::Box.enabled? + setup_box - # Define a class in the namespace via eval - @n.eval("class TestClass; def hello; 'from namespace'; end; end") + # Define a class in the box via eval + @box.eval("class TestClass; def hello; 'from box'; end; end") - # Class should be accessible in the namespace - instance = @n::TestClass.new - assert_equal "from namespace", instance.hello + # Class should be accessible in the box + instance = @box::TestClass.new + assert_equal "from box", instance.hello - # Class should not be visible in main namespace + # Class should not be visible in main box assert_raise(NameError) { TestClass } end def test_eval_isolation - pend unless Ruby::Box.enabled? + setup_box - # Create another namespace - n2 = Namespace.new + # Create another box + n2 = Ruby::Box.new - # Define different constants in each namespace - @n.eval("ISOLATION_TEST = 'first'") + # Define different constants in each box + @box.eval("ISOLATION_TEST = 'first'") n2.eval("ISOLATION_TEST = 'second'") - # Each namespace should have its own constant - assert_equal "first", @n::ISOLATION_TEST + # Each box should have its own constant + assert_equal "first", @box::ISOLATION_TEST assert_equal "second", n2::ISOLATION_TEST # Constants should not interfere with each other - assert_not_equal @n::ISOLATION_TEST, n2::ISOLATION_TEST + assert_not_equal @box::ISOLATION_TEST, n2::ISOLATION_TEST end def test_eval_with_variables - pend unless Ruby::Box.enabled? + setup_box # Test local variable access (should work within the eval context) - result = @n.eval("x = 10; y = 20; x + y") + result = @box.eval("x = 10; y = 20; x + y") assert_equal 30, result end def test_eval_error_handling - pend unless Ruby::Box.enabled? + setup_box # Test syntax error - assert_raise(SyntaxError) { @n.eval("1 +") } + assert_raise(SyntaxError) { @box.eval("1 +") } # Test name error - assert_raise(NameError) { @n.eval("undefined_variable") } + assert_raise(NameError) { @box.eval("undefined_variable") } - # Test that namespace is properly restored after error + # Test that box is properly restored after error begin - @n.eval("raise RuntimeError, 'test error'") + @box.eval("raise RuntimeError, 'test error'") rescue RuntimeError - # Should be able to continue using the namespace - result = @n.eval("2 + 2") + # Should be able to continue using the box + result = @box.eval("2 + 2") assert_equal 4, result end end @@ -702,24 +710,24 @@ def test_root_and_main_methods def test_basic_box_detections assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; - ns = Ruby::Box.new + box = Ruby::Box.new $gvar1 = 'bar' code = <<~EOC - NS1 = Ruby::Box.current + BOX1 = Ruby::Box.current $gvar1 = 'foo' def toplevel = $gvar1 class Foo - NS2 = Ruby::Box.current - NS2_proc = ->(){ NS2 } - NS3_proc = ->(){ Ruby::Box.current } - - def ns4 = Ruby::Box.current - def self.ns5 = NS2 - def self.ns6 = Ruby::Box.current - def self.ns6_proc = ->(){ Ruby::Box.current } - def self.ns7 + BOX2 = Ruby::Box.current + BOX2_proc = ->(){ BOX2 } + BOX3_proc = ->(){ Ruby::Box.current } + + def box4 = Ruby::Box.current + def self.box5 = BOX2 + def self.box6 = Ruby::Box.current + def self.box6_proc = ->(){ Ruby::Box.current } + def self.box7 res = [] [1,2].chunk{ it.even? }.each do |bool, members| res << Ruby::Box.current.object_id.to_s + ":" + bool.to_s + ":" + members.map(&:to_s).join(",") @@ -740,35 +748,35 @@ def foo_box = Ruby::Box.current module_function :foo_box end - NS_X = Foo.new.ns4 - NS_Y = foo_box + BOX_X = Foo.new.box4 + BOX_Y = foo_box EOC - ns.eval(code) + box.eval(code) outer = Ruby::Box.current - assert_equal ns, ns::NS1 # on TOP frame - assert_equal ns, ns::Foo::NS2 # on CLASS frame - assert_equal ns, ns::Foo::NS2_proc.call # proc -> a const on CLASS - assert_equal ns, ns::Foo::NS3_proc.call # proc -> the current - assert_equal ns, ns::Foo.new.ns4 # instance method -> the current - assert_equal ns, ns::Foo.ns5 # singleton method -> a const on CLASS - assert_equal ns, ns::Foo.ns6 # singleton method -> the current - assert_equal ns, ns::Foo.ns6_proc.call # method returns a proc -> the current + assert_equal box, box::BOX1 # on TOP frame + assert_equal box, box::Foo::BOX2 # on CLASS frame + assert_equal box, box::Foo::BOX2_proc.call # proc -> a const on CLASS + assert_equal box, box::Foo::BOX3_proc.call # proc -> the current + assert_equal box, box::Foo.new.box4 # instance method -> the current + assert_equal box, box::Foo.box5 # singleton method -> a const on CLASS + assert_equal box, box::Foo.box6 # singleton method -> the current + assert_equal box, box::Foo.box6_proc.call # method returns a proc -> the current # a block after CFUNC/IFUNC in a method -> the current - assert_equal ["#{ns.object_id}:false:1", "#{ns.object_id}:true:2"], ns::Foo.ns7 + assert_equal ["#{box.object_id}:false:1", "#{box.object_id}:true:2"], box::Foo.box7 - assert_equal outer, ns::Foo.yield_block{ Ruby::Box.current } # method yields - assert_equal outer, ns::Foo.call_block{ Ruby::Box.current } # method calls a block + assert_equal outer, box::Foo.yield_block{ Ruby::Box.current } # method yields + assert_equal outer, box::Foo.call_block{ Ruby::Box.current } # method calls a block - assert_equal 'foo', ns::Foo.gvar1 # method refers gvar - assert_equal 'bar', $gvar1 # gvar value out of the ns - assert_equal 'foo', ns::Foo.call_toplevel # toplevel method referring gvar + assert_equal 'foo', box::Foo.gvar1 # method refers gvar + assert_equal 'bar', $gvar1 # gvar value out of the box + assert_equal 'foo', box::Foo.call_toplevel # toplevel method referring gvar - assert_equal ns, ns::NS_X # on TOP frame, referring a class in the current - assert_equal ns, ns::NS_Y # on TOP frame, referring Kernel method defined by a CFUNC method + assert_equal box, box::BOX_X # on TOP frame, referring a class in the current + assert_equal box, box::BOX_Y # on TOP frame, referring Kernel method defined by a CFUNC method - assert_equal "Foo", ns::FOO_NAME - assert_equal "Foo", ns::Foo.name + assert_equal "Foo", box::FOO_NAME + assert_equal "Foo", box::Foo.name end; end From 28b195fc67788c03be59c2a4cbf0cad52ac3b90f Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sun, 14 Dec 2025 10:46:52 +0100 Subject: [PATCH 7/9] Store the fiber_serial in the EC to allow inlining MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutexes spend a significant amount of time in `rb_fiber_serial` because it can't be inlined (except with LTO). The fiber struct is opaque the so function can't be defined as inlineable. Ideally the while fiber struct would not be opaque to the rest of Ruby core, but it's tricky to do. Instead we can store the fiber serial in the execution context itself, and make its access cheaper: ``` $ hyperfine './miniruby-baseline --yjit /tmp/mut.rb' './miniruby-inline-serial --yjit /tmp/mut.rb' Benchmark 1: ./miniruby-baseline --yjit /tmp/mut.rb Time (mean ± σ): 4.011 s ± 0.084 s [User: 3.977 s, System: 0.011 s] Range (min … max): 3.950 s … 4.245 s 10 runs Benchmark 2: ./miniruby-inline-serial --yjit /tmp/mut.rb Time (mean ± σ): 3.495 s ± 0.150 s [User: 3.448 s, System: 0.009 s] Range (min … max): 3.340 s … 3.869 s 10 runs Summary ./miniruby-inline-serial --yjit /tmp/mut.rb ran 1.15 ± 0.05 times faster than ./miniruby-baseline --yjit /tmp/mut.rb ``` ```ruby i = 10_000_000 mut = Mutex.new while i > 0 i -= 1 mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } mut.synchronize { } end ``` --- cont.c | 13 ++----------- internal/cont.h | 7 ++++++- thread.c | 8 ++++---- thread_sync.c | 49 +++++++++++++++++++++++++------------------------ vm_core.h | 1 + 5 files changed, 38 insertions(+), 40 deletions(-) diff --git a/cont.c b/cont.c index 8d5efeaac431a1..404574e110c870 100644 --- a/cont.c +++ b/cont.c @@ -268,8 +268,6 @@ struct rb_fiber_struct { unsigned int killed : 1; - rb_serial_t serial; - struct coroutine_context context; struct fiber_pool_stack stack; }; @@ -1012,13 +1010,6 @@ rb_fiber_threadptr(const rb_fiber_t *fiber) return fiber->cont.saved_ec.thread_ptr; } -rb_serial_t -rb_fiber_serial(const rb_fiber_t *fiber) -{ - VM_ASSERT(fiber->serial >= 1); - return fiber->serial; -} - static VALUE cont_thread_value(const rb_context_t *cont) { @@ -2026,10 +2017,10 @@ fiber_t_alloc(VALUE fiber_value, unsigned int blocking) fiber->cont.type = FIBER_CONTEXT; fiber->blocking = blocking; fiber->killed = 0; - fiber->serial = next_fiber_serial(th->ractor); cont_init(&fiber->cont, th); fiber->cont.saved_ec.fiber_ptr = fiber; + fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor); rb_ec_clear_vm_stack(&fiber->cont.saved_ec); fiber->prev = NULL; @@ -2576,10 +2567,10 @@ rb_threadptr_root_fiber_setup(rb_thread_t *th) } fiber->cont.type = FIBER_CONTEXT; fiber->cont.saved_ec.fiber_ptr = fiber; + fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor); fiber->cont.saved_ec.thread_ptr = th; fiber->blocking = 1; fiber->killed = 0; - fiber->serial = next_fiber_serial(th->ractor); fiber_status_set(fiber, FIBER_RESUMED); /* skip CREATED */ th->ec = &fiber->cont.saved_ec; cont_init_jit_cont(&fiber->cont); diff --git a/internal/cont.h b/internal/cont.h index 21a054f37c1294..bbcaf0b18824c4 100644 --- a/internal/cont.h +++ b/internal/cont.h @@ -31,6 +31,11 @@ VALUE rb_fiber_inherit_storage(struct rb_execution_context_struct *ec, struct rb VALUE rb_fiberptr_self(struct rb_fiber_struct *fiber); unsigned int rb_fiberptr_blocking(struct rb_fiber_struct *fiber); struct rb_execution_context_struct * rb_fiberptr_get_ec(struct rb_fiber_struct *fiber); -rb_serial_t rb_fiber_serial(const struct rb_fiber_struct *fiber); +static inline rb_serial_t +rb_ec_fiber_serial(struct rb_execution_context_struct *ec) +{ + VM_ASSERT(ec->fiber_serial >= 1); + return ec->fiber_serial; +} #endif /* INTERNAL_CONT_H */ diff --git a/thread.c b/thread.c index e9d36cb1f69bc0..cea8e7fe75d362 100644 --- a/thread.c +++ b/thread.c @@ -454,7 +454,7 @@ rb_threadptr_unlock_all_locking_mutexes(rb_thread_t *th) // rb_warn("mutex #<%p> was not unlocked by thread #<%p>", (void *)mutex, (void*)th); VM_ASSERT(mutex->fiber_serial); - const char *error_message = rb_mutex_unlock_th(mutex, th, NULL); + const char *error_message = rb_mutex_unlock_th(mutex, th, 0); if (error_message) rb_bug("invalid keeping_mutexes: %s", error_message); } } @@ -5283,7 +5283,7 @@ rb_thread_shield_owned(VALUE self) rb_mutex_t *m = mutex_ptr(mutex); - return m->fiber_serial == rb_fiber_serial(GET_EC()->fiber_ptr); + return m->fiber_serial == rb_ec_fiber_serial(GET_EC()); } /* @@ -5302,7 +5302,7 @@ rb_thread_shield_wait(VALUE self) if (!mutex) return Qfalse; m = mutex_ptr(mutex); - if (m->fiber_serial == rb_fiber_serial(GET_EC()->fiber_ptr)) return Qnil; + if (m->fiber_serial == rb_ec_fiber_serial(GET_EC())) return Qnil; rb_thread_shield_waiting_inc(self); rb_mutex_lock(mutex); rb_thread_shield_waiting_dec(self); @@ -5860,7 +5860,7 @@ rb_check_deadlock(rb_ractor_t *r) } else if (th->locking_mutex) { rb_mutex_t *mutex = mutex_ptr(th->locking_mutex); - if (mutex->fiber_serial == rb_fiber_serial(th->ec->fiber_ptr) || (!mutex->fiber_serial && !ccan_list_empty(&mutex->waitq))) { + if (mutex->fiber_serial == rb_ec_fiber_serial(th->ec) || (!mutex->fiber_serial && !ccan_list_empty(&mutex->waitq))) { found = 1; } } diff --git a/thread_sync.c b/thread_sync.c index 6af6aaddd6241d..5af86c5a3b294d 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -81,7 +81,7 @@ static void rb_mutex_abandon_all(rb_mutex_t *mutexes); static void rb_mutex_abandon_keeping_mutexes(rb_thread_t *th); static void rb_mutex_abandon_locking_mutex(rb_thread_t *th); #endif -static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber); +static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial); /* * Document-class: Thread::Mutex @@ -133,7 +133,7 @@ mutex_free(void *ptr) { rb_mutex_t *mutex = ptr; if (mutex_locked_p(mutex)) { - const char *err = rb_mutex_unlock_th(mutex, rb_thread_ptr(mutex->thread), NULL); + const char *err = rb_mutex_unlock_th(mutex, rb_thread_ptr(mutex->thread), 0); if (err) rb_bug("%s", err); } ruby_xfree(ptr); @@ -221,26 +221,26 @@ thread_mutex_remove(rb_thread_t *thread, rb_mutex_t *mutex) } static void -mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber) +mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) { mutex->thread = th->self; - mutex->fiber_serial = rb_fiber_serial(fiber); + mutex->fiber_serial = fiber_serial; } static void -mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber) +mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) { - mutex_set_owner(mutex, th, fiber); + mutex_set_owner(mutex, th, fiber_serial); thread_mutex_insert(th, mutex); } static inline bool -mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber) +mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) { if (mutex->fiber_serial == 0) { RUBY_DEBUG_LOG("%p ok", mutex); - mutex_locked(mutex, th, fiber); + mutex_locked(mutex, th, fiber_serial); return true; } else { @@ -252,7 +252,7 @@ mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber) static VALUE rb_mut_trylock(rb_execution_context_t *ec, VALUE self) { - return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, ec->fiber_ptr)); + return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, rb_ec_fiber_serial(ec))); } VALUE @@ -262,9 +262,9 @@ rb_mutex_trylock(VALUE self) } static VALUE -mutex_owned_p(rb_fiber_t *fiber, rb_mutex_t *mutex) +mutex_owned_p(rb_serial_t fiber_serial, rb_mutex_t *mutex) { - return RBOOL(mutex->fiber_serial == rb_fiber_serial(fiber)); + return RBOOL(mutex->fiber_serial == fiber_serial); } static VALUE @@ -305,6 +305,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_execution_context_t *ec = args->ec; rb_thread_t *th = ec->thread_ptr; rb_fiber_t *fiber = ec->fiber_ptr; + rb_serial_t fiber_serial = rb_ec_fiber_serial(ec); rb_mutex_t *mutex = args->mutex; rb_atomic_t saved_ints = 0; @@ -314,12 +315,12 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_raise(rb_eThreadError, "can't be called from trap context"); } - if (!mutex_trylock(mutex, th, fiber)) { - if (mutex->fiber_serial == rb_fiber_serial(fiber)) { + if (!mutex_trylock(mutex, th, fiber_serial)) { + if (mutex->fiber_serial == fiber_serial) { rb_raise(rb_eThreadError, "deadlock; recursive locking"); } - while (mutex->fiber_serial != rb_fiber_serial(fiber)) { + while (mutex->fiber_serial != fiber_serial) { VM_ASSERT(mutex->fiber_serial != 0); VALUE scheduler = rb_fiber_scheduler_current(); @@ -335,7 +336,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_ensure(call_rb_fiber_scheduler_block, self, delete_from_waitq, (VALUE)&sync_waiter); if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber); + mutex_set_owner(mutex, th, fiber_serial); } } else { @@ -376,7 +377,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) // unlocked by another thread while sleeping if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber); + mutex_set_owner(mutex, th, fiber_serial); } rb_ractor_sleeper_threads_dec(th->ractor); @@ -389,13 +390,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) if (interruptible_p) { /* release mutex before checking for interrupts...as interrupt checking * code might call rb_raise() */ - if (mutex->fiber_serial == rb_fiber_serial(fiber)) { + if (mutex->fiber_serial == fiber_serial) { mutex->thread = Qfalse; mutex->fiber_serial = 0; } RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */ if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber); + mutex_set_owner(mutex, th, fiber_serial); } } else { @@ -414,13 +415,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) } if (saved_ints) th->ec->interrupt_flag = saved_ints; - if (mutex->fiber_serial == rb_fiber_serial(fiber)) mutex_locked(mutex, th, fiber); + if (mutex->fiber_serial == fiber_serial) mutex_locked(mutex, th, fiber_serial); } RUBY_DEBUG_LOG("%p locked", mutex); // assertion - if (mutex_owned_p(fiber, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned."); + if (mutex_owned_p(fiber_serial, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned."); return self; } @@ -455,7 +456,7 @@ rb_mutex_lock(VALUE self) static VALUE rb_mut_owned_p(rb_execution_context_t *ec, VALUE self) { - return mutex_owned_p(ec->fiber_ptr, mutex_ptr(self)); + return mutex_owned_p(rb_ec_fiber_serial(ec), mutex_ptr(self)); } VALUE @@ -465,14 +466,14 @@ rb_mutex_owned_p(VALUE self) } static const char * -rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_fiber_t *fiber) +rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) { RUBY_DEBUG_LOG("%p", mutex); if (mutex->fiber_serial == 0) { return "Attempt to unlock a mutex which is not locked"; } - else if (fiber && mutex->fiber_serial != rb_fiber_serial(fiber)) { + else if (fiber_serial && mutex->fiber_serial != fiber_serial) { return "Attempt to unlock a mutex which is locked by another thread/fiber"; } @@ -516,7 +517,7 @@ do_mutex_unlock(struct mutex_args *args) rb_mutex_t *mutex = args->mutex; rb_thread_t *th = rb_ec_thread_ptr(args->ec); - err = rb_mutex_unlock_th(mutex, th, args->ec->fiber_ptr); + err = rb_mutex_unlock_th(mutex, th, rb_ec_fiber_serial(args->ec)); if (err) rb_raise(rb_eThreadError, "%s", err); } diff --git a/vm_core.h b/vm_core.h index d391c4258a4337..d61686f4b66bff 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1041,6 +1041,7 @@ struct rb_execution_context_struct { rb_fiber_t *fiber_ptr; struct rb_thread_struct *thread_ptr; + rb_serial_t fiber_serial; /* storage (ec (fiber) local) */ struct rb_id_table *local_storage; From e42bcd7ce76e75601ef3adf35467edf277471af2 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 16 Dec 2025 00:43:41 +0100 Subject: [PATCH 8/9] Rename fiber_serial into ec_serial Since it now live in the EC. --- cont.c | 8 +++--- internal/cont.h | 7 ----- ractor.c | 4 +-- ractor_core.h | 2 +- thread.c | 10 +++---- thread_sync.c | 70 ++++++++++++++++++++++++------------------------- vm_core.h | 9 ++++++- 7 files changed, 55 insertions(+), 55 deletions(-) diff --git a/cont.c b/cont.c index 404574e110c870..0087994730419e 100644 --- a/cont.c +++ b/cont.c @@ -1996,9 +1996,9 @@ fiber_alloc(VALUE klass) } static rb_serial_t -next_fiber_serial(rb_ractor_t *cr) +next_ec_serial(rb_ractor_t *cr) { - return cr->next_fiber_serial++; + return cr->next_ec_serial++; } static rb_fiber_t* @@ -2020,7 +2020,7 @@ fiber_t_alloc(VALUE fiber_value, unsigned int blocking) cont_init(&fiber->cont, th); fiber->cont.saved_ec.fiber_ptr = fiber; - fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor); + fiber->cont.saved_ec.serial = next_ec_serial(th->ractor); rb_ec_clear_vm_stack(&fiber->cont.saved_ec); fiber->prev = NULL; @@ -2567,7 +2567,7 @@ rb_threadptr_root_fiber_setup(rb_thread_t *th) } fiber->cont.type = FIBER_CONTEXT; fiber->cont.saved_ec.fiber_ptr = fiber; - fiber->cont.saved_ec.fiber_serial = next_fiber_serial(th->ractor); + fiber->cont.saved_ec.serial = next_ec_serial(th->ractor); fiber->cont.saved_ec.thread_ptr = th; fiber->blocking = 1; fiber->killed = 0; diff --git a/internal/cont.h b/internal/cont.h index bbcaf0b18824c4..dcf6f820a38f87 100644 --- a/internal/cont.h +++ b/internal/cont.h @@ -31,11 +31,4 @@ VALUE rb_fiber_inherit_storage(struct rb_execution_context_struct *ec, struct rb VALUE rb_fiberptr_self(struct rb_fiber_struct *fiber); unsigned int rb_fiberptr_blocking(struct rb_fiber_struct *fiber); struct rb_execution_context_struct * rb_fiberptr_get_ec(struct rb_fiber_struct *fiber); - -static inline rb_serial_t -rb_ec_fiber_serial(struct rb_execution_context_struct *ec) -{ - VM_ASSERT(ec->fiber_serial >= 1); - return ec->fiber_serial; -} #endif /* INTERNAL_CONT_H */ diff --git a/ractor.c b/ractor.c index f65a4f79c1f8e8..5c98196c912fa2 100644 --- a/ractor.c +++ b/ractor.c @@ -422,7 +422,7 @@ ractor_alloc(VALUE klass) VALUE rv = TypedData_Make_Struct(klass, rb_ractor_t, &ractor_data_type, r); FL_SET_RAW(rv, RUBY_FL_SHAREABLE); r->pub.self = rv; - r->next_fiber_serial = 1; + r->next_ec_serial = 1; VM_ASSERT(ractor_status_p(r, ractor_created)); return rv; } @@ -440,7 +440,7 @@ rb_ractor_main_alloc(void) r->name = Qnil; r->pub.self = Qnil; r->newobj_cache = rb_gc_ractor_cache_alloc(r); - r->next_fiber_serial = 1; + r->next_ec_serial = 1; ruby_single_main_ractor = r; return r; diff --git a/ractor_core.h b/ractor_core.h index d6f9d4eda0fd4b..96d22bea29eade 100644 --- a/ractor_core.h +++ b/ractor_core.h @@ -91,7 +91,7 @@ struct rb_ractor_struct { // ractor local data - rb_serial_t next_fiber_serial; + rb_serial_t next_ec_serial; st_table *local_storage; struct rb_id_table *idkey_local_storage; diff --git a/thread.c b/thread.c index cea8e7fe75d362..3e1bb1dbe7ad17 100644 --- a/thread.c +++ b/thread.c @@ -453,7 +453,7 @@ rb_threadptr_unlock_all_locking_mutexes(rb_thread_t *th) th->keeping_mutexes = mutex->next_mutex; // rb_warn("mutex #<%p> was not unlocked by thread #<%p>", (void *)mutex, (void*)th); - VM_ASSERT(mutex->fiber_serial); + VM_ASSERT(mutex->ec_serial); const char *error_message = rb_mutex_unlock_th(mutex, th, 0); if (error_message) rb_bug("invalid keeping_mutexes: %s", error_message); } @@ -5283,7 +5283,7 @@ rb_thread_shield_owned(VALUE self) rb_mutex_t *m = mutex_ptr(mutex); - return m->fiber_serial == rb_ec_fiber_serial(GET_EC()); + return m->ec_serial == rb_ec_serial(GET_EC()); } /* @@ -5302,7 +5302,7 @@ rb_thread_shield_wait(VALUE self) if (!mutex) return Qfalse; m = mutex_ptr(mutex); - if (m->fiber_serial == rb_ec_fiber_serial(GET_EC())) return Qnil; + if (m->ec_serial == rb_ec_serial(GET_EC())) return Qnil; rb_thread_shield_waiting_inc(self); rb_mutex_lock(mutex); rb_thread_shield_waiting_dec(self); @@ -5820,7 +5820,7 @@ debug_deadlock_check(rb_ractor_t *r, VALUE msg) if (th->locking_mutex) { rb_mutex_t *mutex = mutex_ptr(th->locking_mutex); rb_str_catf(msg, " mutex:%llu cond:%"PRIuSIZE, - (unsigned long long)mutex->fiber_serial, rb_mutex_num_waiting(mutex)); + (unsigned long long)mutex->ec_serial, rb_mutex_num_waiting(mutex)); } { @@ -5860,7 +5860,7 @@ rb_check_deadlock(rb_ractor_t *r) } else if (th->locking_mutex) { rb_mutex_t *mutex = mutex_ptr(th->locking_mutex); - if (mutex->fiber_serial == rb_ec_fiber_serial(th->ec) || (!mutex->fiber_serial && !ccan_list_empty(&mutex->waitq))) { + if (mutex->ec_serial == rb_ec_serial(th->ec) || (!mutex->ec_serial && !ccan_list_empty(&mutex->waitq))) { found = 1; } } diff --git a/thread_sync.c b/thread_sync.c index 5af86c5a3b294d..6bff982f31d78b 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -7,7 +7,7 @@ static VALUE rb_eClosedQueueError; /* Mutex */ typedef struct rb_mutex_struct { - rb_serial_t fiber_serial; + rb_serial_t ec_serial; VALUE thread; // even if the fiber is collected, we might need access to the thread in mutex_free struct rb_mutex_struct *next_mutex; struct ccan_list_head waitq; /* protected by GVL */ @@ -81,7 +81,7 @@ static void rb_mutex_abandon_all(rb_mutex_t *mutexes); static void rb_mutex_abandon_keeping_mutexes(rb_thread_t *th); static void rb_mutex_abandon_locking_mutex(rb_thread_t *th); #endif -static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial); +static const char* rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial); /* * Document-class: Thread::Mutex @@ -125,7 +125,7 @@ rb_thread_t* rb_fiber_threadptr(const rb_fiber_t *fiber); static bool mutex_locked_p(rb_mutex_t *mutex) { - return mutex->fiber_serial != 0; + return mutex->ec_serial != 0; } static void @@ -221,26 +221,26 @@ thread_mutex_remove(rb_thread_t *thread, rb_mutex_t *mutex) } static void -mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) +mutex_set_owner(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) { mutex->thread = th->self; - mutex->fiber_serial = fiber_serial; + mutex->ec_serial = ec_serial; } static void -mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) +mutex_locked(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) { - mutex_set_owner(mutex, th, fiber_serial); + mutex_set_owner(mutex, th, ec_serial); thread_mutex_insert(th, mutex); } static inline bool -mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) +mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) { - if (mutex->fiber_serial == 0) { + if (mutex->ec_serial == 0) { RUBY_DEBUG_LOG("%p ok", mutex); - mutex_locked(mutex, th, fiber_serial); + mutex_locked(mutex, th, ec_serial); return true; } else { @@ -252,7 +252,7 @@ mutex_trylock(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) static VALUE rb_mut_trylock(rb_execution_context_t *ec, VALUE self) { - return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, rb_ec_fiber_serial(ec))); + return RBOOL(mutex_trylock(mutex_ptr(self), ec->thread_ptr, rb_ec_serial(ec))); } VALUE @@ -262,9 +262,9 @@ rb_mutex_trylock(VALUE self) } static VALUE -mutex_owned_p(rb_serial_t fiber_serial, rb_mutex_t *mutex) +mutex_owned_p(rb_serial_t ec_serial, rb_mutex_t *mutex) { - return RBOOL(mutex->fiber_serial == fiber_serial); + return RBOOL(mutex->ec_serial == ec_serial); } static VALUE @@ -305,7 +305,7 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_execution_context_t *ec = args->ec; rb_thread_t *th = ec->thread_ptr; rb_fiber_t *fiber = ec->fiber_ptr; - rb_serial_t fiber_serial = rb_ec_fiber_serial(ec); + rb_serial_t ec_serial = rb_ec_serial(ec); rb_mutex_t *mutex = args->mutex; rb_atomic_t saved_ints = 0; @@ -315,13 +315,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_raise(rb_eThreadError, "can't be called from trap context"); } - if (!mutex_trylock(mutex, th, fiber_serial)) { - if (mutex->fiber_serial == fiber_serial) { + if (!mutex_trylock(mutex, th, ec_serial)) { + if (mutex->ec_serial == ec_serial) { rb_raise(rb_eThreadError, "deadlock; recursive locking"); } - while (mutex->fiber_serial != fiber_serial) { - VM_ASSERT(mutex->fiber_serial != 0); + while (mutex->ec_serial != ec_serial) { + VM_ASSERT(mutex->ec_serial != 0); VALUE scheduler = rb_fiber_scheduler_current(); if (scheduler != Qnil) { @@ -335,8 +335,8 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) rb_ensure(call_rb_fiber_scheduler_block, self, delete_from_waitq, (VALUE)&sync_waiter); - if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber_serial); + if (!mutex->ec_serial) { + mutex_set_owner(mutex, th, ec_serial); } } else { @@ -376,8 +376,8 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) ccan_list_del(&sync_waiter.node); // unlocked by another thread while sleeping - if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber_serial); + if (!mutex->ec_serial) { + mutex_set_owner(mutex, th, ec_serial); } rb_ractor_sleeper_threads_dec(th->ractor); @@ -390,13 +390,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) if (interruptible_p) { /* release mutex before checking for interrupts...as interrupt checking * code might call rb_raise() */ - if (mutex->fiber_serial == fiber_serial) { + if (mutex->ec_serial == ec_serial) { mutex->thread = Qfalse; - mutex->fiber_serial = 0; + mutex->ec_serial = 0; } RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */ - if (!mutex->fiber_serial) { - mutex_set_owner(mutex, th, fiber_serial); + if (!mutex->ec_serial) { + mutex_set_owner(mutex, th, ec_serial); } } else { @@ -415,13 +415,13 @@ do_mutex_lock(struct mutex_args *args, int interruptible_p) } if (saved_ints) th->ec->interrupt_flag = saved_ints; - if (mutex->fiber_serial == fiber_serial) mutex_locked(mutex, th, fiber_serial); + if (mutex->ec_serial == ec_serial) mutex_locked(mutex, th, ec_serial); } RUBY_DEBUG_LOG("%p locked", mutex); // assertion - if (mutex_owned_p(fiber_serial, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned."); + if (mutex_owned_p(ec_serial, mutex) == Qfalse) rb_bug("do_mutex_lock: mutex is not owned."); return self; } @@ -456,7 +456,7 @@ rb_mutex_lock(VALUE self) static VALUE rb_mut_owned_p(rb_execution_context_t *ec, VALUE self) { - return mutex_owned_p(rb_ec_fiber_serial(ec), mutex_ptr(self)); + return mutex_owned_p(rb_ec_serial(ec), mutex_ptr(self)); } VALUE @@ -466,20 +466,20 @@ rb_mutex_owned_p(VALUE self) } static const char * -rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t fiber_serial) +rb_mutex_unlock_th(rb_mutex_t *mutex, rb_thread_t *th, rb_serial_t ec_serial) { RUBY_DEBUG_LOG("%p", mutex); - if (mutex->fiber_serial == 0) { + if (mutex->ec_serial == 0) { return "Attempt to unlock a mutex which is not locked"; } - else if (fiber_serial && mutex->fiber_serial != fiber_serial) { + else if (ec_serial && mutex->ec_serial != ec_serial) { return "Attempt to unlock a mutex which is locked by another thread/fiber"; } struct sync_waiter *cur = 0, *next; - mutex->fiber_serial = 0; + mutex->ec_serial = 0; thread_mutex_remove(th, mutex); ccan_list_for_each_safe(&mutex->waitq, cur, next, node) { @@ -517,7 +517,7 @@ do_mutex_unlock(struct mutex_args *args) rb_mutex_t *mutex = args->mutex; rb_thread_t *th = rb_ec_thread_ptr(args->ec); - err = rb_mutex_unlock_th(mutex, th, rb_ec_fiber_serial(args->ec)); + err = rb_mutex_unlock_th(mutex, th, rb_ec_serial(args->ec)); if (err) rb_raise(rb_eThreadError, "%s", err); } @@ -583,7 +583,7 @@ rb_mutex_abandon_all(rb_mutex_t *mutexes) while (mutexes) { mutex = mutexes; mutexes = mutex->next_mutex; - mutex->fiber_serial = 0; + mutex->ec_serial = 0; mutex->next_mutex = 0; ccan_list_head_init(&mutex->waitq); } diff --git a/vm_core.h b/vm_core.h index d61686f4b66bff..c716e001a5f542 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1041,7 +1041,7 @@ struct rb_execution_context_struct { rb_fiber_t *fiber_ptr; struct rb_thread_struct *thread_ptr; - rb_serial_t fiber_serial; + rb_serial_t serial; /* storage (ec (fiber) local) */ struct rb_id_table *local_storage; @@ -2037,6 +2037,13 @@ RUBY_EXTERN unsigned int ruby_vm_event_local_num; #define GET_THREAD() rb_current_thread() #define GET_EC() rb_current_execution_context(true) +static inline rb_serial_t +rb_ec_serial(struct rb_execution_context_struct *ec) +{ + VM_ASSERT(ec->serial >= 1); + return ec->serial; +} + static inline rb_thread_t * rb_ec_thread_ptr(const rb_execution_context_t *ec) { From 2b1a9afbfbb5491665bfbdd560486a310ca49755 Mon Sep 17 00:00:00 2001 From: Misaki Shioi <31817032+shioimm@users.noreply.github.com> Date: Tue, 16 Dec 2025 19:52:45 +0900 Subject: [PATCH 9/9] Fix: Do not pass negative timeout to Addrinfo#connect_internal (#15578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change fixes a bug where, with `Socket.tcp`’s `fast_fallback option` disabled, specifying `open_timeout` could unintentionally pass a negative value to `Addrinfo#connect_internal`, `causing an ArgumentError`. ``` ❯ ruby -rsocket -e 'p Socket.tcp("localhost", 9292, open_timeout: 1, fast_fallback: false)' /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:64:in 'IO#wait_writable': time interval must not be negative (ArgumentError) sock.wait_writable(timeout) or ^^^^^^^ from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:64:in 'Addrinfo#connect_internal' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:141:in 'Addrinfo#connect' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:964:in 'block in Socket.tcp_without_fast_fallback' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:231:in 'Array#each' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:231:in 'Addrinfo.foreach' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:945:in 'Socket.tcp_without_fast_fallback' from /Users/misaki-shioi/src/install/lib/ruby/4.0.0+0/socket.rb:671:in 'Socket.tcp' from -e:1:in '
' ``` --- ext/socket/lib/socket.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ext/socket/lib/socket.rb b/ext/socket/lib/socket.rb index 9862c92c0b6591..f47b5bc1d17a54 100644 --- a/ext/socket/lib/socket.rb +++ b/ext/socket/lib/socket.rb @@ -948,7 +948,14 @@ def self.tcp_without_fast_fallback(host, port, local_host, local_port, connect_t local_addr = nil end begin - timeout = open_timeout ? open_timeout - (current_clock_time - starts_at) : connect_timeout + timeout = + if open_timeout + t = open_timeout - (current_clock_time - starts_at) + t.negative? ? 0 : t + else + connect_timeout + end + sock = local_addr ? ai.connect_from(local_addr, timeout:) : ai.connect(timeout:)