diff --git a/.github/workflows/auto_request_review.yml b/.github/workflows/auto_request_review.yml index 80f2517eb5dd35..9b816020e1fae5 100644 --- a/.github/workflows/auto_request_review.yml +++ b/.github/workflows/auto_request_review.yml @@ -14,7 +14,7 @@ jobs: if: ${{ github.repository == 'ruby/ruby' && github.base_ref == 'master' }} steps: - name: Request review based on files changes and/or groups the author belongs to - uses: necojackarc/auto-request-review@035f049cb68460341ab744f19aa9f31aae685e36 # master + uses: necojackarc/auto-request-review@9a4b11888f2d60c79120c5413f13eb63febc2b68 # master with: # scope: public_repo token: ${{ secrets.MATZBOT_AUTO_REQUEST_REVIEW_TOKEN }} diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 0f265d0b36b8bf..4655487c4ef8c5 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -112,3 +112,18 @@ jobs: -H "X-GitHub-Api-Version: 2022-11-28" \ https://api.github.com/repos/ruby/all-ruby/dispatches \ -d '{"event_type": "update"}' + + - uses: ruby/action-slack@d260b61aa817726d5bedd22dd6cc305787fa4cdd # v4.0.0 + with: + payload: | + { + "attachments": [{ + "title": "Publish Ruby ${{ github.event.client_payload.version || github.event.inputs.version }}", + "title_link": "https://cache.ruby-lang.org/pub/ruby/", + "text": "${{ job.status }}: ", + "color": "${{ job.status == 'success' && 'good' || 'danger' }}" + }] + } + env: + SLACK_WEBHOOK_URL: ${{ secrets.SNAPSHOT_SLACK_WEBHOOK_URL }} + if: always() diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 8ee5f12ecead35..df6140edecffce 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@9534c84618278caac52cb373bb164ed464dbd8af # v2.87.11 + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 8e0ebddea901b3..8be42652d5450f 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -152,7 +152,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@9534c84618278caac52cb373bb164ed464dbd8af # v2.87.11 + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/ext/socket/ipsocket.c b/ext/socket/ipsocket.c index 506a56a1865bc4..fecbc3ef8e241a 100644 --- a/ext/socket/ipsocket.c +++ b/ext/socket/ipsocket.c @@ -314,7 +314,7 @@ allocate_fast_fallback_getaddrinfo_shared(int family_size) } static void -allocate_fast_fallback_getaddrinfo_hints(struct addrinfo *hints, int family, int remote_addrinfo_hints, int additional_flags) +init_fast_fallback_getaddrinfo_hints(struct addrinfo *hints, int family, int remote_addrinfo_hints, int additional_flags) { MEMZERO(hints, struct addrinfo, 1); hints->ai_family = family; @@ -691,7 +691,7 @@ init_fast_fallback_inetsock_internal(VALUE v) struct addrinfo getaddrinfo_hints[arg->family_size]; - allocate_fast_fallback_getaddrinfo_hints( + init_fast_fallback_getaddrinfo_hints( &getaddrinfo_hints[i], arg->families[i], remote_addrinfo_hints, @@ -707,18 +707,18 @@ init_fast_fallback_inetsock_internal(VALUE v) arg->getaddrinfo_entries[i]->test_ecode = 0; /* for testing HEv2 */ - if (!NIL_P(test_mode_settings) && RB_TYPE_P(test_mode_settings, T_HASH)) { + if (RB_TYPE_P(test_mode_settings, T_HASH)) { const char *family_sym = arg->families[i] == AF_INET6 ? "ipv6" : "ipv4"; VALUE test_delay_setting = rb_hash_aref(test_mode_settings, ID2SYM(rb_intern("delay"))); - if (!NIL_P(test_delay_setting)) { + if (RB_TYPE_P(test_delay_setting, T_HASH)) { VALUE rb_test_delay_ms = rb_hash_aref(test_delay_setting, ID2SYM(rb_intern(family_sym))); long test_delay_ms = NIL_P(rb_test_delay_ms) ? 0 : NUM2LONG(rb_test_delay_ms); arg->getaddrinfo_entries[i]->test_sleep_ms = test_delay_ms; } VALUE test_error_setting = rb_hash_aref(test_mode_settings, ID2SYM(rb_intern("error"))); - if (!NIL_P(test_error_setting)) { + if (RB_TYPE_P(test_error_setting, T_HASH)) { VALUE rb_test_ecode = rb_hash_aref(test_error_setting, ID2SYM(rb_intern(family_sym))); if (!NIL_P(rb_test_ecode)) { arg->getaddrinfo_entries[i]->test_ecode = NUM2INT(rb_test_ecode); diff --git a/file.c b/file.c index 0cf60f44da0b61..7bdaebc311a026 100644 --- a/file.c +++ b/file.c @@ -2885,7 +2885,7 @@ rb_file_ctime(VALUE obj) * */ -VALUE +static VALUE rb_file_s_birthtime(VALUE klass, VALUE fname) { rb_io_stat_data st; diff --git a/lib/bundler/gem_helper.rb b/lib/bundler/gem_helper.rb index e3af1b957f357c..ea02e7e7712bbe 100644 --- a/lib/bundler/gem_helper.rb +++ b/lib/bundler/gem_helper.rb @@ -216,7 +216,7 @@ def sh_with_status(cmd, &block) Bundler.ui.debug(cmd) SharedHelpers.chdir(base) do outbuf = IO.popen(cmd, err: [:child, :out], &:read) - status = $? + status = Process.last_status block&.call(outbuf) if status.success? [outbuf, status] end diff --git a/lib/rubygems/ext/builder.rb b/lib/rubygems/ext/builder.rb index b7e80d6b06bb89..691bc2825da015 100644 --- a/lib/rubygems/ext/builder.rb +++ b/lib/rubygems/ext/builder.rb @@ -101,7 +101,9 @@ def self.run(command, results, command_name = nil, dir = Dir.pwd, env = {}) require "open3" # Set $SOURCE_DATE_EPOCH for the subprocess. - # Under Ruby::Box mkmf makes RbConfig.expand recurse until SystemStackError. + # Under Ruby::Box defined?($gvar) does not see assignments made inside the + # box, so mkmf have_devel? never memoizes and recurses until SystemStackError + # (https://bugs.ruby-lang.org/issues/22283). # Drop $RUBY_BOX last so no caller can restore it. build_env = { "SOURCE_DATE_EPOCH" => Gem.source_date_epoch_string }.merge(env).merge("RUBY_BOX" => nil) # A single-element command would be parsed as a shell command line, diff --git a/lib/rubygems/source/git.rb b/lib/rubygems/source/git.rb index baf2f9dd4c0932..0ceb67b382f578 100644 --- a/lib/rubygems/source/git.rb +++ b/lib/rubygems/source/git.rb @@ -188,9 +188,11 @@ def rev_parse # :nodoc: hash = Gem::Util.popen(git_command, "rev-parse", @reference).strip end + # Process.last_status instead of $?, which Ruby::Box leaves uninitialized + # (https://bugs.ruby-lang.org/issues/22280) raise Gem::Exception, "unable to find reference #{@reference} in #{@repository}" unless - $?.success? + Process.last_status.success? hash end diff --git a/spec/bundler/bundler/plugin_spec.rb b/spec/bundler/bundler/plugin_spec.rb index 1e10036b63c24f..5f5054b6842513 100644 --- a/spec/bundler/bundler/plugin_spec.rb +++ b/spec/bundler/bundler/plugin_spec.rb @@ -318,6 +318,8 @@ end it "executes the hook" do + skip "Ruby::Box ignores $stdout reassignment (https://bugs.ruby-lang.org/issues/21867)" if defined?(Ruby::Box) && Ruby::Box.enabled? + expect do Plugin.hook(Bundler::Plugin::Events::EVENT1) end.to output("hook for event 1\n").to_stdout @@ -331,6 +333,8 @@ RUBY it "evals plugins.rb once" do + skip "Ruby::Box ignores $stdout reassignment (https://bugs.ruby-lang.org/issues/21867)" if defined?(Ruby::Box) && Ruby::Box.enabled? + expect do Plugin.hook(Bundler::Plugin::Events::EVENT1) Plugin.hook(Bundler::Plugin::Events::EVENT2) @@ -344,6 +348,8 @@ RUBY it "is passed to the hook" do + skip "Ruby::Box ignores $stdout reassignment (https://bugs.ruby-lang.org/issues/21867)" if defined?(Ruby::Box) && Ruby::Box.enabled? + expect do Plugin.hook(Bundler::Plugin::Events::EVENT1) { puts "win" } end.to output("win\n").to_stdout diff --git a/spec/bundler/bundler/shared_helpers_spec.rb b/spec/bundler/bundler/shared_helpers_spec.rb index 1619b0a14a5982..232da4143a2429 100644 --- a/spec/bundler/bundler/shared_helpers_spec.rb +++ b/spec/bundler/bundler/shared_helpers_spec.rb @@ -387,7 +387,11 @@ before do ENV["RUBYOPT"] = "-r#{install_path}/bundler/setup" - allow(File).to receive(:expand_path).and_return("#{install_path}/bundler/setup") + # Only fake the resolution of bundler/setup itself. A blanket stub + # breaks unrelated RubyGems path lookups triggered lazily inside the + # example, see #set_rubyopt. + allow(File).to receive(:expand_path).and_call_original + allow(File).to receive(:expand_path).with("setup", anything).and_return("#{install_path}/bundler/setup") allow(Gem).to receive(:bin_path).and_return("#{install_path}/bundler/setup") end @@ -403,7 +407,8 @@ let(:install_path) { "/opt/ruby with space/lib" } before do - allow(File).to receive(:expand_path).and_return("#{install_path}/bundler/setup") + allow(File).to receive(:expand_path).and_call_original + allow(File).to receive(:expand_path).with("setup", anything).and_return("#{install_path}/bundler/setup") allow(Gem).to receive(:bin_path).and_return("#{install_path}/bundler/setup") end diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb index d4b88902e21e28..b5e4a9aa76c75c 100644 --- a/spec/bundler/commands/install_spec.rb +++ b/spec/bundler/commands/install_spec.rb @@ -2065,6 +2065,10 @@ def gem_make_out end it "preserves bundled native extensions when BUNDLE_CLEAN removes another gem" do + # The command-line/RUBYOPT -r bypasses gem activation under RUBY_BOX=1 + # (https://bugs.ruby-lang.org/issues/22295) + skip "-r cannot activate gems under Ruby::Box" if defined?(Ruby::Box) && Ruby::Box.enabled? + build_repo4 do build_gem "native_child", "1.0", &:add_c_extension build_gem "native_parent", "1.0" do |s| diff --git a/spec/bundler/install/gemfile/content_addressable_spec.rb b/spec/bundler/install/gemfile/content_addressable_spec.rb index 8493c89cc20381..8b0a472413fcdc 100644 --- a/spec/bundler/install/gemfile/content_addressable_spec.rb +++ b/spec/bundler/install/gemfile/content_addressable_spec.rb @@ -509,6 +509,7 @@ RSpec.describe "bundle install with content-addressable gems invisible to pre-4.1 RubyGems clients", :compact_index, rubygems: ">= 4.1.0.a" do before do skip "Gem::ContentAddress not available" if ruby_core? + skip "A prerelease Ruby does not satisfy the ~> X.Y.0 ABI pin of content-addressed gems" if Gem.ruby_version.prerelease? end let(:current_abi) { "#{Gem.ruby_version.segments[0]}.#{Gem.ruby_version.segments[1]}" } diff --git a/spec/bundler/runtime/env_helpers_spec.rb b/spec/bundler/runtime/env_helpers_spec.rb index 38f501e7cde6aa..0ebf86f29606d3 100644 --- a/spec/bundler/runtime/env_helpers_spec.rb +++ b/spec/bundler/runtime/env_helpers_spec.rb @@ -167,13 +167,13 @@ def run_bundler_script(env, script) create_file("source.rb", <<-'RUBY') Bundler.original_system("ruby", "-e", "exit(42) if ENV['BUNDLE_FOO'] == 'bar'") - exit $?.exitstatus + exit Process.last_status.exitstatus RUBY end it "runs system inside with_original_env" do run_bundler_script({ "BUNDLE_FOO" => "bar" }, bundled_app("source.rb")) - expect($?.exitstatus).to eq(42) + expect(Process.last_status.exitstatus).to eq(42) end end @@ -182,13 +182,13 @@ def run_bundler_script(env, script) create_file("source.rb", <<-'RUBY') Bundler.unbundled_system("ruby", "-e", "exit(42) unless ENV['BUNDLE_FOO'] == 'bar'") - exit $?.exitstatus + exit Process.last_status.exitstatus RUBY end it "runs system inside with_unbundled_env" do run_bundler_script({ "BUNDLE_FOO" => "bar" }, bundled_app("source.rb")) - expect($?.exitstatus).to eq(42) + expect(Process.last_status.exitstatus).to eq(42) end end @@ -209,7 +209,7 @@ def run_bundler_script(env, script) skip "Fork not implemented" if Gem.win_platform? run_bundler_script({ "BUNDLE_FOO" => "bar" }, bundled_app("source.rb")) - expect($?.exitstatus).to eq(0) + expect(Process.last_status.exitstatus).to eq(0) end end @@ -230,7 +230,7 @@ def run_bundler_script(env, script) skip "Fork not implemented" if Gem.win_platform? run_bundler_script({ "BUNDLE_FOO" => "bar" }, bundled_app("source.rb")) - expect($?.exitstatus).to eq(1) + expect(Process.last_status.exitstatus).to eq(1) end end end diff --git a/spec/bundler/support/command_execution.rb b/spec/bundler/support/command_execution.rb index e2915b996d9d3d..14da54dca4da90 100644 --- a/spec/bundler/support/command_execution.rb +++ b/spec/bundler/support/command_execution.rb @@ -2,6 +2,13 @@ module Spec class CommandExecution + # Under RUBY_BOX, every spawned ruby prints an experimental warning to + # stderr, breaking specs that assert clean stderr. + RUBY_BOX_WARNING = Regexp.union( + /^[^\n]*: warning: Ruby::Box is experimental, and the behavior may change in the future!\n?/, + %r{^See https://docs\.ruby-lang\.org/\S+ for known issues, etc\.\n?} + ) + def initialize(command, timeout:) @command = command @timeout = timeout @@ -72,7 +79,13 @@ def failure? attr_reader :failure_reason def normalize(string) - string.dup.force_encoding(Encoding::UTF_8).scrub.strip.gsub("\r\n", "\n") + string = string.dup.force_encoding(Encoding::UTF_8).scrub.gsub("\r\n", "\n") + string = string.gsub(RUBY_BOX_WARNING, "") if ruby_box_enabled? + string.strip + end + + def ruby_box_enabled? + defined?(Ruby::Box) && Ruby::Box.enabled? end end end diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 09b7f427ebbf1f..38ac67be3add16 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -404,6 +404,10 @@ def setup ENV["BUNDLE_COOLDOWN"] = nil ENV["RUBYGEMS_PREVENT_UPDATE_SUGGESTION"] = "true" + # Child ruby processes inherit RUBY_BOX and print an experimental + # warning on startup, breaking assertions on subprocess stderr. + ENV["RUBYOPT"] = [ENV["RUBYOPT"], "-W:no-experimental"].compact.join(" ") if ruby_box_enabled? + @current_dir = Dir.pwd @fetcher = nil @@ -1440,6 +1444,23 @@ def ruby_repo? !ENV["GEM_COMMAND"].nil? end + ## + # Is this test running under Ruby::Box (RUBY_BOX=1)? + + def ruby_box_enabled? + defined?(Ruby::Box) && Ruby::Box.enabled? + end + + ## + # Ruby::Box gives each box detached copies of the stdio globals, so + # reassigning $stdout/$stderr cannot capture output written by Kernel#warn, + # Kernel#puts or subprocesses. Pends until the ruby-core fix for + # https://bugs.ruby-lang.org/issues/21867 lands. + + def pend_for_ruby_box_stdio_capture + pend "Ruby::Box breaks $stdout/$stderr capture (https://bugs.ruby-lang.org/issues/21867)" if ruby_box_enabled? + end + ## # Returns the make command for the current platform. For versions of Ruby # built on MS Windows with VC++ or Borland it will return 'nmake'. On all diff --git a/test/rubygems/test_deprecate.rb b/test/rubygems/test_deprecate.rb index bb6a0b5ceaaf38..5700d356e7562e 100644 --- a/test/rubygems/test_deprecate.rb +++ b/test/rubygems/test_deprecate.rb @@ -132,6 +132,7 @@ def test_deprecated_method_calls_the_old_method end def test_deprecated_method_outputs_a_warning + pend_for_ruby_box_stdio_capture out, err = capture_output do thing = Thing.new thing.foo @@ -165,6 +166,7 @@ def execute end def test_deprecated_method_outputs_a_warning_old_way + pend_for_ruby_box_stdio_capture out, err = capture_output do thing = OtherThing.new thing.foo @@ -180,6 +182,7 @@ def test_deprecated_method_outputs_a_warning_old_way end def test_deprecated_method_when_class_overrides_format + pend_for_ruby_box_stdio_capture out, err = capture_output do thing = ThingWithFormat.new thing.foo diff --git a/test/rubygems/test_exit.rb b/test/rubygems/test_exit.rb index 396837edadfa3a..c339c39a67af62 100644 --- a/test/rubygems/test_exit.rb +++ b/test/rubygems/test_exit.rb @@ -6,7 +6,8 @@ class TestGemExit < Gem::TestCase def test_exit system(*ruby_with_rubygems_in_load_path, "-e", "raise Gem::SystemExitException.new(2)") - assert_equal 2, $?.exitstatus + # Process.last_status instead of $?, which Ruby::Box leaves uninitialized + assert_equal 2, Process.last_status.exitstatus end def test_status diff --git a/test/rubygems/test_gem.rb b/test/rubygems/test_gem.rb index 88b461a2a0a74f..36067549c86370 100644 --- a/test/rubygems/test_gem.rb +++ b/test/rubygems/test_gem.rb @@ -1297,6 +1297,7 @@ def test_self_try_activate_missing_prerelease end def test_self_try_activate_missing_extensions + pend_for_ruby_box_stdio_capture spec = util_spec "ext", "1" do |s| s.extensions = %w[ext/extconf.rb] s.installed_by_version = v("2.2") @@ -1352,6 +1353,7 @@ def test_setting_paths_does_not_mutate_parameter_object end def test_deprecated_paths= + pend_for_ruby_box_stdio_capture stdout, stderr = capture_output do Gem.paths = { "GEM_HOME" => Gem.paths.home, "GEM_PATH" => [Gem.paths.home, "foo"] } diff --git a/test/rubygems/test_gem_commands_build_command.rb b/test/rubygems/test_gem_commands_build_command.rb index 771eb07dbc9cdd..5bbb7c3b3e356e 100644 --- a/test/rubygems/test_gem_commands_build_command.rb +++ b/test/rubygems/test_gem_commands_build_command.rb @@ -383,6 +383,7 @@ def test_execute_strict_with_warnings end def test_execute_bad_spec + pend_for_ruby_box_stdio_capture @gem.date = "2010-11-08" gemspec_file = File.join(@tempdir, @gem.spec_name) diff --git a/test/rubygems/test_gem_commands_open_command.rb b/test/rubygems/test_gem_commands_open_command.rb index 3a774a9343c08b..c30117a58ecc22 100644 --- a/test/rubygems/test_gem_commands_open_command.rb +++ b/test/rubygems/test_gem_commands_open_command.rb @@ -21,6 +21,7 @@ def gem(name, version = "1.0") end def test_execute + pend_for_ruby_box_stdio_capture omit "JRuby on Windows spawns the editor with a different cwd" if Gem.win_platform? && Gem.java_platform? @cmd.options[:args] = %w[foo] diff --git a/test/rubygems/test_gem_config_file.rb b/test/rubygems/test_gem_config_file.rb index 2c33192a4b3a83..7120c49e327afb 100644 --- a/test/rubygems/test_gem_config_file.rb +++ b/test/rubygems/test_gem_config_file.rb @@ -314,6 +314,7 @@ def test_handle_arguments_backtrace end def test_handle_arguments_debug + pend_for_ruby_box_stdio_capture assert_equal false, $DEBUG args = %w[--debug] diff --git a/test/rubygems/test_gem_doctor.rb b/test/rubygems/test_gem_doctor.rb index 1554e7af128dd4..da625000ff9010 100644 --- a/test/rubygems/test_gem_doctor.rb +++ b/test/rubygems/test_gem_doctor.rb @@ -240,6 +240,7 @@ def test_doctor_preserves_valid_abi_scoped_gemspec end def test_doctor_removes_corrupt_abi_scoped_gemspec + pend_for_ruby_box_stdio_capture install_specs util_spec "regular_gem" spec = util_ca_spec "ca_gem", "1", "aabbccdd", @@ -286,6 +287,7 @@ def test_doctor_preserves_other_abi_dir end def test_doctor_does_not_recurse_into_abi_symlink + pend_for_ruby_box_stdio_capture pend "symlinks not supported" unless symlink_supported? install_specs util_spec "regular_gem" diff --git a/test/rubygems/test_gem_ext_builder.rb b/test/rubygems/test_gem_ext_builder.rb index 6b4eed2cf21175..f5ad791ec1c904 100644 --- a/test/rubygems/test_gem_ext_builder.rb +++ b/test/rubygems/test_gem_ext_builder.rb @@ -700,7 +700,7 @@ def self.expand(val, config = CONFIG); val; end system(Gem.ruby, "-rmkmf", "-e", "exit MakeMakefile::RbConfig::CONFIG['host_os'] == 'fake_os'", "--", "--target-rbconfig=#{fake_rbconfig}") end - unless $?.success? + unless Process.last_status.success? assert_include(stderr, "uninitialized constant MakeMakefile::RbConfig") pend "This version of mkmf does not support --target-rbconfig" end diff --git a/test/rubygems/test_gem_ext_cargo_builder.rb b/test/rubygems/test_gem_ext_cargo_builder.rb index b970e442c25084..bf442125f27548 100644 --- a/test/rubygems/test_gem_ext_cargo_builder.rb +++ b/test/rubygems/test_gem_ext_cargo_builder.rb @@ -111,7 +111,9 @@ def test_full_integration Open3.capture2e(*gem, "build", "rust_ruby_example.gemspec", "--output", built_gem) Open3.capture2e(*gem, "install", "--verbose", "--local", built_gem, *ARGV) - stdout_and_stderr_str, status = Open3.capture2e(env_for_subprocess, *ruby_with_rubygems_in_load_path, "-rrust_ruby_example", "-e", "puts 'Result: ' + RustRubyExample.reverse('hello world')") + # Require inside -e because -r bypasses gem activation under RUBY_BOX=1 + # (https://bugs.ruby-lang.org/issues/22295) + stdout_and_stderr_str, status = Open3.capture2e(env_for_subprocess, *ruby_with_rubygems_in_load_path, "-e", "require 'rust_ruby_example'; puts 'Result: ' + RustRubyExample.reverse('hello world')") assert status.success?, stdout_and_stderr_str assert_match "Result: #{"hello world".reverse}", stdout_and_stderr_str end @@ -134,7 +136,7 @@ def test_custom_name Open3.capture2e(*gem, "install", "--verbose", "--local", built_gem, *ARGV) end - stdout_and_stderr_str, status = Open3.capture2e(env_for_subprocess, *ruby_with_rubygems_in_load_path, "-rcustom_name", "-e", "puts 'Result: ' + CustomName.say_hello") + stdout_and_stderr_str, status = Open3.capture2e(env_for_subprocess, *ruby_with_rubygems_in_load_path, "-e", "require 'custom_name'; puts 'Result: ' + CustomName.say_hello") assert status.success?, stdout_and_stderr_str assert_match "Result: Hello world!", stdout_and_stderr_str @@ -199,7 +201,7 @@ def skip_unsupported_platforms! pend "jruby not supported" if Gem.java_platform? pend "truffleruby not supported (yet)" if RUBY_ENGINE == "truffleruby" system(@rust_envs, "cargo", "-V", out: IO::NULL, err: [:child, :out]) - pend "cargo not present" unless $?.success? + pend "cargo not present" unless Process.last_status.success? pend "ruby.h is not provided by ruby repo" if ruby_repo? pend "rust toolchain of mingw is broken" if mingw_windows? end diff --git a/test/rubygems/test_gem_package.rb b/test/rubygems/test_gem_package.rb index b0935693d1c7e6..4e83b3a69b467b 100644 --- a/test/rubygems/test_gem_package.rb +++ b/test/rubygems/test_gem_package.rb @@ -1538,6 +1538,7 @@ def test_verify_corrupt end def test_verify_corrupt_tar_metadata_entry + pend_for_ruby_box_stdio_capture gem = tar_file_header("metadata.gz", "", 0, 999, Time.now) File.open "corrupt.gem", "wb" do |io| @@ -1574,6 +1575,7 @@ def test_verify_corrupt_tar_checksums_entry end def test_verify_corrupt_tar_data_entry + pend_for_ruby_box_stdio_capture gem = tar_file_header("data.tar.gz", "", 0, 100, Time.now) File.open "corrupt.gem", "wb" do |io| diff --git a/test/rubygems/test_gem_package_tar_header_ractor.rb b/test/rubygems/test_gem_package_tar_header_ractor.rb index 57140648052ee8..d2fc0f69b1e939 100644 --- a/test/rubygems/test_gem_package_tar_header_ractor.rb +++ b/test/rubygems/test_gem_package_tar_header_ractor.rb @@ -8,6 +8,11 @@ end class TestGemPackageTarHeaderRactor < Gem::Package::TarTestCase + def setup + super + pend "Ruby::Box ignores $VERBOSE=, so assert_ractor cannot keep the Ractor experimental warning out of the child stderr (https://bugs.ruby-lang.org/issues/22282)" if ruby_box_enabled? + end + SETUP = <<~RUBY header = { name: "x", diff --git a/test/rubygems/test_gem_request_set.rb b/test/rubygems/test_gem_request_set.rb index 8c8be04fb9f9a0..60ff8724aef45a 100644 --- a/test/rubygems/test_gem_request_set.rb +++ b/test/rubygems/test_gem_request_set.rb @@ -71,6 +71,7 @@ def test_install_from_gemdeps end def test_install_from_gemdeps_explain + pend_for_ruby_box_stdio_capture spec_fetcher do |fetcher| fetcher.gem "a", 2 end @@ -94,6 +95,7 @@ def test_install_from_gemdeps_explain end def test_install_from_gemdeps_explain_verbose + pend_for_ruby_box_stdio_capture spec_fetcher do |fetcher| fetcher.gem "a", 2 end diff --git a/test/rubygems/test_gem_request_set_gem_dependency_api.rb b/test/rubygems/test_gem_request_set_gem_dependency_api.rb index 4b5eaa38eda8ef..d8f4e7f6e92b4f 100644 --- a/test/rubygems/test_gem_request_set_gem_dependency_api.rb +++ b/test/rubygems/test_gem_request_set_gem_dependency_api.rb @@ -78,6 +78,7 @@ def test_gem end def test_gem_duplicate + pend_for_ruby_box_stdio_capture @gda.gem "a" _, err = capture_output do @@ -128,6 +129,7 @@ def test_gem_bitbucket_expand_path end def test_gem_git_branch + pend_for_ruby_box_stdio_capture _, err = capture_output do @gda.gem "a", git: "git/a", branch: "other", tag: "v1" end @@ -149,6 +151,7 @@ def test_gem_git_gist end def test_gem_git_ref + pend_for_ruby_box_stdio_capture _, err = capture_output do @gda.gem "a", git: "git/a", ref: "abcd123", branch: "other" end diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index dc32a62907863c..0f107d12529a3e 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -1576,6 +1576,7 @@ def test_contains_requirable_file_eh end def test_contains_requirable_file_eh_extension + pend_for_ruby_box_stdio_capture ext_spec _, err = capture_output do @@ -3386,6 +3387,7 @@ def test_validate_files end def test_unresolved_specs + pend_for_ruby_box_stdio_capture specification = Gem::Specification.clone set_orig specification @@ -3412,6 +3414,7 @@ def test_unresolved_specs end def test_unresolved_specs_with_versions + pend_for_ruby_box_stdio_capture specification = Gem::Specification.clone set_orig specification @@ -3444,6 +3447,7 @@ def test_unresolved_specs_with_versions end def test_unresolved_specs_with_duplicated_versions + pend_for_ruby_box_stdio_capture specification = Gem::Specification.clone set_orig specification @@ -3497,6 +3501,7 @@ def test_unresolved_specs_with_unrestricted_deps_on_default_gems end def test_duplicate_runtime_dependency + pend_for_ruby_box_stdio_capture expected = "WARNING: duplicated b dependency [\"~> 3.0\", \"~> 3.0\"]\n" out, err = capture_output do @a1.add_dependency "b", "~> 3.0", "~> 3.0" diff --git a/test/rubygems/test_gem_stub_specification.rb b/test/rubygems/test_gem_stub_specification.rb index 1aa3b6532436ec..66bdd3d3fbbb23 100644 --- a/test/rubygems/test_gem_stub_specification.rb +++ b/test/rubygems/test_gem_stub_specification.rb @@ -94,6 +94,7 @@ def test_contains_requirable_file_eh end def test_contains_requirable_file_eh_extension + pend_for_ruby_box_stdio_capture stub_with_extension do |stub| _, err = capture_output do if RUBY_ENGINE == "jruby" diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb index db86a309056575..ef1bb2e465a22b 100644 --- a/test/rubygems/test_require.rb +++ b/test/rubygems/test_require.rb @@ -484,7 +484,7 @@ def test_realworld_default_gem puts Gem.loaded_specs["json"] RUBY output = Gem::Util.popen(*ruby_with_rubygems_in_load_path, "-e", cmd).strip - assert $?.success? + assert Process.last_status.success? refute_empty output end @@ -508,7 +508,7 @@ def test_realworld_upgraded_default_gem assert_equal "999.99.9", output.lines[0].chomp # Make sure only files from the newer json gem are loaded, and no files from the default json gem assert_equal ["#{@gemhome}/gems/json-999.99.9/lib/json.rb"], output.lines.grep(%r{/gems/json-}).map(&:chomp) - assert $?.success? + assert Process.last_status.success? end def test_default_gem_and_normal_gem @@ -718,6 +718,7 @@ def test_require_bundler ["", "Kernel."].each do |prefix| define_method "test_no_kernel_require_in_#{prefix.tr(".", "_")}warn_with_uplevel" do + pend_for_ruby_box_stdio_capture Dir.mktmpdir("warn_test") do |dir| File.write(dir + "/sub.rb", "#{prefix}warn 'uplevel', 'test', uplevel: 1\n") File.write(dir + "/main.rb", "require 'sub'\n") @@ -733,6 +734,7 @@ def test_require_bundler end define_method "test_no_other_behavioral_changes_with_#{prefix.tr(".", "_")}warn" do + pend_for_ruby_box_stdio_capture Dir.mktmpdir("warn_test") do |dir| File.write(dir + "/main.rb", "#{prefix}warn({x:1}, {y:2}, [])\n") _, err = capture_subprocess_io do @@ -748,6 +750,7 @@ def test_require_bundler end def test_no_crash_when_overriding_warn_with_warning_module + pend_for_ruby_box_stdio_capture Dir.mktmpdir("warn_test") do |dir| File.write(dir + "/main.rb", "module Warning; def warn(str); super; end; end; warn 'Foo Bar'") _, err = capture_subprocess_io do @@ -762,6 +765,7 @@ def test_no_crash_when_overriding_warn_with_warning_module end def test_expected_backtrace_location_when_inheriting_from_basic_object_and_including_kernel + pend_for_ruby_box_stdio_capture Dir.mktmpdir("warn_test") do |dir| File.write(dir + "/main.rb", "\nrequire 'sub'\n") File.write(dir + "/sub.rb", <<-'RUBY') diff --git a/test/rubygems/test_rubygems.rb b/test/rubygems/test_rubygems.rb index 6566b5981e691b..02393c57fcaeec 100644 --- a/test/rubygems/test_rubygems.rb +++ b/test/rubygems/test_rubygems.rb @@ -5,7 +5,7 @@ class GemTest < Gem::TestCase def test_rubygems_normal_behaviour _ = Gem::Util.popen(*ruby_with_rubygems_in_load_path, "-e", "'require \"rubygems\"'", { err: [:child, :out] }).strip - assert $?.success? + assert Process.last_status.success? end def test_operating_system_other_exceptions @@ -17,7 +17,7 @@ def test_operating_system_other_exceptions RUBY output = Gem::Util.popen(*ruby_with_rubygems_and_fake_operating_system_in_load_path(path), "-e", "'require \"rubygems\"'", { err: [:child, :out] }).strip - assert !$?.success? + assert !Process.last_status.success? assert_match(/undefined local variable or method [`']intentionally_not_implemented_method'/, output) assert_includes output, "Loading the #{operating_system_rb_at(path)} file caused an error. " \ "This file is owned by your OS, not by rubygems upstream. " \ diff --git a/test/socket/test_tcp.rb b/test/socket/test_tcp.rb index cfa1413d3699f8..0c1e3cc4e397ba 100644 --- a/test/socket/test_tcp.rb +++ b/test/socket/test_tcp.rb @@ -180,7 +180,8 @@ def test_initialize_v6_hostname_resolved_earlier return end - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } port = server.addr[1] socket = TCPSocket.new( @@ -191,7 +192,8 @@ def test_initialize_v6_hostname_resolved_earlier ) assert_true(socket.remote_address.ipv6?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end @@ -202,7 +204,8 @@ def test_initialize_v4_hostname_resolved_earlier server = TCPServer.new("127.0.0.1", 0) port = server.addr[1] - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } socket = TCPSocket.new( "localhost", port, @@ -211,7 +214,8 @@ def test_initialize_v4_hostname_resolved_earlier ) assert_true(socket.remote_address.ipv4?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end @@ -311,7 +315,8 @@ def test_initialize_v6_hostname_resolution_failed_and_v4_hostname_resolution_is_ server = TCPServer.new("127.0.0.1", 0) port = server.addr[1] - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } socket = TCPSocket.new( "localhost", port, @@ -320,7 +325,30 @@ def test_initialize_v6_hostname_resolution_failed_and_v4_hostname_resolution_is_ ) assert_true(socket.remote_address.ipv4?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close + server&.close + socket&.close + end + + def test_initialize_ignores_malformed_test_mode_settings + return if RUBY_PLATFORM =~ /mswin|mingw|cygwin/ + + server = TCPServer.new("127.0.0.1", 0) + port = server.addr[1] + + accepted = nil + server_thread = Thread.new { accepted = server.accept } + socket = TCPSocket.new( + "localhost", + port, + fast_fallback: true, + test_mode_settings: { delay: 1, error: 1 } + ) + assert_equal(port, socket.remote_address.ip_port) + ensure + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end @@ -396,13 +424,15 @@ def test_initialize_v6_connected_socket_with_v6_address return end - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } port = server.addr[1] socket = TCPSocket.new("::1", port) assert_true(socket.remote_address.ipv6?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end @@ -411,13 +441,15 @@ def test_initialize_v4_connected_socket_with_v4_address return if RUBY_PLATFORM =~ /mswin|mingw|cygwin/ server = TCPServer.new("127.0.0.1", 0) - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } port = server.addr[1] socket = TCPSocket.new("127.0.0.1", port) assert_true(socket.remote_address.ipv4?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end @@ -427,12 +459,14 @@ def test_initialize_fast_fallback_is_false server = TCPServer.new("127.0.0.1", 0) _, port, = server.addr - server_thread = Thread.new { server.accept } + accepted = nil + server_thread = Thread.new { accepted = server.accept } socket = TCPSocket.new("127.0.0.1", port, fast_fallback: false) assert_true(socket.remote_address.ipv4?) ensure - server_thread&.value&.close + stop_accept_thread(server_thread, socket) + accepted&.close server&.close socket&.close end diff --git a/thread.c b/thread.c index 2a64366ec79e0d..479f703b487da9 100644 --- a/thread.c +++ b/thread.c @@ -214,7 +214,7 @@ static inline int vm_check_ints_blocking(rb_execution_context_t *ec) { #ifdef RUBY_ASSERT_CRITICAL_SECTION - VM_ASSERT(ruby_assert_critical_section_entered == 0); + VM_ASSERT(ec->assert_critical_section_entered == 0); #endif rb_thread_t *th = rb_ec_thread_ptr(ec); @@ -1704,7 +1704,7 @@ blocking_region_begin(rb_thread_t *th, struct rb_blocking_region_buffer *region, rb_unblock_function_t *ubf, void *arg, int flags) { #ifdef RUBY_ASSERT_CRITICAL_SECTION - VM_ASSERT(ruby_assert_critical_section_entered == 0); + VM_ASSERT(th->ec->assert_critical_section_entered == 0); #endif VM_ASSERT(th == GET_THREAD()); diff --git a/variable.c b/variable.c index 2d4659637e0bc6..9eab776a5547e6 100644 --- a/variable.c +++ b/variable.c @@ -599,29 +599,73 @@ rb_free_generic_fields_tbl_(void) st_free_table(generic_fields_tbl_); } +static void +rb_gvar_undef_compactor(void *var) +{ +} + +NORETURN(static void global_entry_isolation_error(ID id)); + +static void +global_entry_isolation_error(ID id) +{ + rb_raise(rb_eRactorIsolationError, "can not access global variable %s from non-main Ractor", rb_id2name(id)); +} + +/* Sets *isolation_error when the caller must raise; the caller has to do that + * once it no longer holds the VM lock. */ static struct rb_global_entry* -rb_find_global_entry(ID id) +global_entry_lookup(ID id, bool create_entry, bool *isolation_error) { struct rb_global_entry *entry; VALUE data; RB_VM_LOCKING() { - if (!rb_id_table_lookup(rb_global_tbl, id, &data)) { - entry = NULL; - } - else { + if (rb_id_table_lookup(rb_global_tbl, id, &data)) { entry = (struct rb_global_entry *)data; RUBY_ASSERT(entry != NULL); } - } + else { + entry = NULL; + } + + *isolation_error = UNLIKELY(!rb_ractor_main_p()) && (!entry || !entry->ractor_local); + + if (!entry && create_entry && !*isolation_error) { + struct rb_global_variable *var = ALLOC(struct rb_global_variable); + entry = ALLOC(struct rb_global_entry); + entry->id = id; + entry->var = var; + entry->ractor_local = false; + var->counter = 1; + var->data = 0; + var->getter = rb_gvar_undef_getter; + var->setter = rb_gvar_undef_setter; + var->marker = rb_gvar_undef_marker; + var->compactor = rb_gvar_undef_compactor; - if (UNLIKELY(!rb_ractor_main_p()) && (!entry || !entry->ractor_local)) { - rb_raise(rb_eRactorIsolationError, "can not access global variable %s from non-main Ractor", rb_id2name(id)); + var->block_trace = 0; + var->trace = 0; + var->box_ready = false; + var->box_dynamic = false; + rb_id_table_insert(rb_global_tbl, id, (VALUE)entry); + } } return entry; } +static struct rb_global_entry* +rb_find_global_entry(ID id) +{ + bool isolation_error; + struct rb_global_entry *entry = global_entry_lookup(id, false, &isolation_error); + + if (isolation_error) global_entry_isolation_error(id); + + return entry; +} + void rb_gvar_ractor_local(const char *name) { @@ -643,38 +687,14 @@ rb_gvar_box_dynamic(const char *name) entry->var->box_dynamic = true; } -static void -rb_gvar_undef_compactor(void *var) -{ -} - static struct rb_global_entry* rb_global_entry(ID id) { - struct rb_global_entry *entry; - RB_VM_LOCKING() { - entry = rb_find_global_entry(id); - if (!entry) { - struct rb_global_variable *var; - entry = ALLOC(struct rb_global_entry); - var = ALLOC(struct rb_global_variable); - entry->id = id; - entry->var = var; - entry->ractor_local = false; - var->counter = 1; - var->data = 0; - var->getter = rb_gvar_undef_getter; - var->setter = rb_gvar_undef_setter; - var->marker = rb_gvar_undef_marker; - var->compactor = rb_gvar_undef_compactor; + bool isolation_error; + struct rb_global_entry *entry = global_entry_lookup(id, true, &isolation_error); + + if (isolation_error) global_entry_isolation_error(id); - var->block_trace = 0; - var->trace = 0; - var->box_ready = false; - var->box_dynamic = false; - rb_id_table_insert(rb_global_tbl, id, (VALUE)entry); - } - } return entry; } @@ -1036,11 +1056,12 @@ rb_gvar_set(ID id, VALUE val) struct rb_global_entry *entry = NULL; const rb_box_t *box = rb_current_box(); bool use_box_tbl = false; + bool isolation_error = false; RB_VM_LOCKING() { - entry = rb_global_entry(id); + entry = global_entry_lookup(id, true, &isolation_error); - if (gvar_use_box_tbl(box, entry)) { + if (!isolation_error && gvar_use_box_tbl(box, entry)) { use_box_tbl = true; rb_hash_aset(box->gvar_tbl, rb_id2sym(entry->id), val); retval = val; @@ -1048,6 +1069,8 @@ rb_gvar_set(ID id, VALUE val) } } + if (isolation_error) global_entry_isolation_error(id); + if (!use_box_tbl) { retval = rb_gvar_set_entry(entry, val); } @@ -1066,33 +1089,41 @@ rb_gvar_get(ID id) VALUE retval, gvars, key; const rb_box_t *box = rb_current_box(); bool use_box_tbl = false; + bool isolation_error = false; struct rb_global_entry *entry = NULL; struct rb_global_variable *var = NULL; - // TODO: use lock-free rb_id_table when it's available for use (doesn't yet exist) - RB_VM_LOCKING() { - entry = rb_global_entry(id); - var = entry->var; - if (gvar_use_box_tbl(box, entry)) { - use_box_tbl = true; - gvars = box->gvar_tbl; - key = rb_id2sym(entry->id); - if (RTEST(rb_hash_has_key(gvars, key))) { // this gvar is already cached - retval = rb_hash_aref(gvars, key); - } - else { - RB_VM_UNLOCK(); - { - retval = (*var->getter)(entry->id, var->data); - if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) { - retval = rb_funcall(retval, rb_intern("clone"), 0); + RB_VM_LOCKING() { + // TODO: use lock-free rb_id_table when it's available for use (doesn't yet exist) + entry = global_entry_lookup(id, true, &isolation_error); + + if (!isolation_error) { + var = entry->var; + + if (gvar_use_box_tbl(box, entry)) { + use_box_tbl = true; + gvars = box->gvar_tbl; + key = rb_id2sym(entry->id); + if (RTEST(rb_hash_has_key(gvars, key))) { // this gvar is already cached + retval = rb_hash_aref(gvars, key); + } + else { + RB_VM_UNLOCK(); + { + retval = (*var->getter)(entry->id, var->data); + if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) { + retval = rb_funcall(retval, rb_intern("clone"), 0); + } } + RB_VM_LOCK(); + rb_hash_aset(gvars, key, retval); } - RB_VM_LOCK(); - rb_hash_aset(gvars, key, retval); } } } + + if (isolation_error) global_entry_isolation_error(id); + if (!use_box_tbl) { retval = (*var->getter)(entry->id, var->data); } @@ -1180,13 +1211,17 @@ rb_alias_variable(ID name1, ID name2) struct rb_global_entry *entry1 = NULL, *entry2; VALUE data1; struct rb_id_table *gtbl = rb_global_tbl; + bool tracer_error = false; if (!rb_ractor_main_p()) { rb_raise(rb_eRactorIsolationError, "can not access global variables from non-main Ractors"); } RB_VM_LOCKING() { - entry2 = rb_global_entry(name2); + bool isolation_error; + entry2 = global_entry_lookup(name2, true, &isolation_error); + VM_ASSERT(!isolation_error); /* main Ractor, checked above */ + if (!rb_id_table_lookup(gtbl, name1, &data1)) { entry1 = ZALLOC(struct rb_global_entry); entry1->id = name1; @@ -1195,19 +1230,22 @@ rb_alias_variable(ID name1, ID name2) else if ((entry1 = (struct rb_global_entry *)data1)->var != entry2->var) { struct rb_global_variable *var = entry1->var; if (var->block_trace) { - RB_VM_UNLOCK(); - rb_raise(rb_eRuntimeError, "can't alias in tracer"); + tracer_error = true; } - var->counter--; - if (var->counter == 0) { - free_global_variable(var); + else { + var->counter--; + if (var->counter == 0) { + free_global_variable(var); + } } } - if (entry1->var != entry2->var) { + if (!tracer_error && entry1->var != entry2->var) { entry2->var->counter++; entry1->var = entry2->var; } } + + if (tracer_error) rb_raise(rb_eRuntimeError, "can't alias in tracer"); } static void diff --git a/vm.c b/vm.c index 426ffcd8a3c875..06da903cf1b073 100644 --- a/vm.c +++ b/vm.c @@ -57,10 +57,6 @@ #include "probes.h" #include "probes_helper.h" -#ifdef RUBY_ASSERT_CRITICAL_SECTION -int ruby_assert_critical_section_entered = 0; -#endif - static void *native_main_thread_stack_top; bool ruby_vm_during_cleanup = false; diff --git a/vm_core.h b/vm_core.h index 4754feae6f6c17..44e19a5463f1d6 100644 --- a/vm_core.h +++ b/vm_core.h @@ -94,9 +94,8 @@ RUBY_ASSERT_CRITICAL_SECTION_LEAVE(); If `rb_vm_check_ints()` is called between the `RUBY_ASSERT_CRITICAL_SECTION_ENTER()` and `RUBY_ASSERT_CRITICAL_SECTION_LEAVE()`, a failed assertion will result. */ -extern int ruby_assert_critical_section_entered; -#define RUBY_ASSERT_CRITICAL_SECTION_ENTER() do{ruby_assert_critical_section_entered += 1;}while(false) -#define RUBY_ASSERT_CRITICAL_SECTION_LEAVE() do{VM_ASSERT(ruby_assert_critical_section_entered > 0);ruby_assert_critical_section_entered -= 1;}while(false) +#define RUBY_ASSERT_CRITICAL_SECTION_ENTER() do{GET_EC()->assert_critical_section_entered += 1;}while(false) +#define RUBY_ASSERT_CRITICAL_SECTION_LEAVE() do{rb_execution_context_t *ec__ = GET_EC();VM_ASSERT(ec__->assert_critical_section_entered > 0);ec__->assert_critical_section_entered -= 1;}while(false) #else #define RUBY_ASSERT_CRITICAL_SECTION_ENTER() #define RUBY_ASSERT_CRITICAL_SECTION_LEAVE() @@ -1147,6 +1146,10 @@ struct rb_execution_context_struct { void *asan_fake_stack_handle; #endif } machine; + +#ifdef RUBY_ASSERT_CRITICAL_SECTION + int assert_critical_section_entered; +#endif }; #ifndef rb_execution_context_t @@ -2363,7 +2366,7 @@ static inline void rb_vm_check_ints(rb_execution_context_t *ec) { #ifdef RUBY_ASSERT_CRITICAL_SECTION - VM_ASSERT(ruby_assert_critical_section_entered == 0); + VM_ASSERT(ec->assert_critical_section_entered == 0); #endif VM_ASSERT(ec == rb_current_ec_noinline()); diff --git a/vm_method.c b/vm_method.c index c8536566a99a66..74acc34cf2816b 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1435,6 +1435,31 @@ rb_zsuper_to_super(int argc, VALUE *argv, VALUE self) } static inline rb_method_entry_t* search_method0(VALUE klass, ID id, VALUE *defined_class_ptr, bool skip_refined); +static void +method_entry_modify_check(VALUE klass, rb_method_type_t type) +{ + ASSERT_vm_unlocking(); + if (type != VM_METHOD_TYPE_REFINED) { + rb_class_modify_check(NIL_P(klass) ? rb_cObject : klass); + } +} + +/* rb_method_entry_make() runs under the VM lock, where it must not warn: + * rb_warn() dispatches Warning.warn and writes to $stderr, either of which can + * check for interrupts. It formats the message instead, and the caller emits + * it once the lock is released. */ +struct method_entry_warnings { + VALUE redefined; /* $VERBOSE only */ + VALUE problem; +}; + +static void +method_entry_warnings_emit(const struct method_entry_warnings *warnings) +{ + if (warnings->redefined) rb_warning("%"PRIsVALUE, warnings->redefined); + if (warnings->problem) rb_warn("%"PRIsVALUE, warnings->problem); +} + /* * klass->method_table[mid] = method_entry(defined_class, visi, def) * @@ -1443,7 +1468,8 @@ static inline rb_method_entry_t* search_method0(VALUE klass, ID id, VALUE *defin */ static rb_method_entry_t * rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibility_t visi, - rb_method_type_t type, rb_method_definition_t *def, ID original_id, void *opts) + rb_method_type_t type, rb_method_definition_t *def, ID original_id, void *opts, + struct method_entry_warnings *warnings) { rb_method_entry_t *me; struct rb_id_table *mtbl; @@ -1470,10 +1496,6 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil } } - if (type != VM_METHOD_TYPE_REFINED) { - rb_class_modify_check(klass); - } - if (RB_TYPE_P(klass, T_MODULE) && FL_TEST(klass, RMODULE_IS_REFINEMENT)) { VALUE refined_class = rb_refinement_module_get_refined_class(klass); if (type == VM_METHOD_TYPE_ZSUPER) { @@ -1524,7 +1546,7 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil break; } if (iseq) { - rb_warning( + warnings->redefined = rb_sprintf( "method redefined; discarding old %"PRIsVALUE"\n%"PRIsVALUE":%d: warning: previous definition of %"PRIsVALUE" was here", rb_id2str(mid), rb_iseq_path(iseq), @@ -1533,7 +1555,7 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil ); } else { - rb_warning("method redefined; discarding old %"PRIsVALUE, rb_id2str(mid)); + warnings->redefined = rb_sprintf("method redefined; discarding old %"PRIsVALUE, rb_id2str(mid)); } } } @@ -1561,13 +1583,13 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil case idRespond_to_missing: case idMethodMissing: case idRespond_to: - rb_warn("redefining Object#%s may cause infinite loop", rb_id2name(mid)); + warnings->problem = rb_sprintf("redefining Object#%s may cause infinite loop", rb_id2name(mid)); } } /* check mid */ if (mid == object_id || mid == id__id__ || mid == id__send__) { if (type != VM_METHOD_TYPE_CFUNC && search_method(klass, mid, 0)) { - rb_warn("redefining '%s' may cause serious problems", rb_id2name(mid)); + warnings->problem = rb_sprintf("redefining '%s' may cause serious problems", rb_id2name(mid)); } } @@ -1733,10 +1755,16 @@ void rb_add_method(VALUE klass, ID mid, rb_method_type_t type, void *opts, rb_method_visibility_t visi) { const rb_method_entry_t *me; + struct method_entry_warnings warnings = {0}; + + method_entry_modify_check(klass, type); + RB_VM_LOCKING() { - me = rb_method_entry_make(klass, mid, klass, visi, type, NULL, mid, opts); + me = rb_method_entry_make(klass, mid, klass, visi, type, NULL, mid, opts, &warnings); } + method_entry_warnings_emit(&warnings); + if (type != VM_METHOD_TYPE_UNDEF && type != VM_METHOD_TYPE_REFINED) { method_added(klass, mid, me); } @@ -1761,15 +1789,21 @@ method_entry_set(VALUE klass, ID mid, const rb_method_entry_t *me, rb_method_visibility_t visi, VALUE defined_class) { rb_method_entry_t *newme; + struct method_entry_warnings warnings = {0}; + + method_entry_modify_check(klass, me->def->type); + RB_VM_LOCKING() { newme = rb_method_entry_make(klass, mid, defined_class, visi, - me->def->type, me->def, 0, NULL); + me->def->type, me->def, 0, NULL, &warnings); if (newme == me) { me->def->no_redef_warning = TRUE; METHOD_ENTRY_FLAGS_SET(newme, visi, FALSE); } } + method_entry_warnings_emit(&warnings); + method_added(klass, mid, newme); return newme; } @@ -2910,7 +2944,7 @@ rb_alias(VALUE klass, ID alias_name, ID original_name) rb_raise(rb_eTypeError, "no class to make alias"); } - rb_class_modify_check(klass); + rb_class_modify_check(target_klass); again: orig_me = search_method(klass, original_name, &defined_class); @@ -2945,10 +2979,14 @@ rb_alias(VALUE klass, ID alias_name, ID original_name) if (visi == METHOD_VISI_UNDEF) visi = METHOD_ENTRY_VISI(orig_me); if (orig_me->defined_class == 0) { + struct method_entry_warnings warnings = {0}; const rb_method_entry_t *alias_me = + // TODO: needs vm lock? rb_method_entry_make(target_klass, alias_name, target_klass, visi, VM_METHOD_TYPE_ALIAS, NULL, orig_me->called_id, - (void *)rb_method_entry_clone(orig_me)); + (void *)rb_method_entry_clone(orig_me), &warnings); + + method_entry_warnings_emit(&warnings); method_added(target_klass, alias_name, alias_me); } else { diff --git a/vm_sync.c b/vm_sync.c index 73335699a01626..35043754e3202e 100644 --- a/vm_sync.c +++ b/vm_sync.c @@ -111,6 +111,7 @@ vm_lock_enter(rb_ractor_t *cr, rb_vm_t *vm, bool locked, bool no_barrier, unsign vm->ractor.sync.lock_rec++; *lev = vm->ractor.sync.lock_rec; + RUBY_ASSERT_CRITICAL_SECTION_ENTER(); RUBY_DEBUG_LOG2(file, line, "rec:%u owner:%u", vm->ractor.sync.lock_rec, (unsigned int)rb_ractor_id(vm->ractor.sync.lock_owner)); @@ -144,6 +145,7 @@ vm_lock_leave(rb_vm_t *vm, bool no_barrier, unsigned int *lev APPEND_LOCATION_AR RUBY_DTRACE_GVL_RELEASE(); } + RUBY_ASSERT_CRITICAL_SECTION_LEAVE(); vm->ractor.sync.lock_rec--; *lev = vm->ractor.sync.lock_rec;