From 42bcc629fba518215c844488223bc279006a4fa2 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Sun, 22 May 2022 00:32:41 +1200 Subject: [PATCH] Retain reference to blocking fibers. --- doc/hacking.md | 33 ++++++++++---------------- test/fiber/scheduler.rb | 14 ++++++----- test/fiber/test_scheduler.rb | 26 ++++++++++---------- variable.c | 46 ++++++++++++------------------------ 4 files changed, 49 insertions(+), 70 deletions(-) diff --git a/doc/hacking.md b/doc/hacking.md index a9fa820e7e..1268f36fb1 100644 --- a/doc/hacking.md +++ b/doc/hacking.md @@ -1,16 +1,12 @@ # Ruby Hacking Guide -This document gives some helpful instructions which should make your -experience as a Ruby core developer easier. +This document gives some helpful instructions which should make your experience as a Ruby core developer easier. ## Setup ### Make -It's common to want to compile things as quickly as possible. Ensuring -`make` has the right `--jobs` flag will ensure all processors are -utilized when building software projects To do this effectively, you -can set `MAKEFLAGS` in your shell configuration/profile: +It's common to want to compile things as quickly as possible. Ensuring `make` has the right `--jobs` flag will ensure all processors are utilized when building software projects To do this effectively, you can set `MAKEFLAGS` in your shell configuration/profile: ``` shell # On macOS with Fish shell: @@ -40,8 +36,7 @@ make install ### Without Documentation -If you are frequently building Ruby, this will reduce the time it -takes to `make install`. +If you are frequently building Ruby, this will reduce the time it takes to `make install`. ``` shell ../configure --disable-install-doc @@ -51,15 +46,13 @@ takes to `make install`. ### Run Local Test Script -You can create a file in the Ruby source root called `test.rb`. You -can build `miniruby` and execute this script: +You can create a file in the Ruby source root called `test.rb`. You can build `miniruby` and execute this script: ``` shell make run ``` -If you want more of the standard library, you can use `runruby` -instead of `run`. +If you want more of the standard library, you can use `runruby` instead of `run`. ## Running Tests @@ -71,8 +64,7 @@ make check ### Run Bootstrap Tests -There are a set of tests in `bootstraptest/` which cover most basic -features of the core Ruby language. +There are a set of tests in `bootstraptest/` which cover most basic features of the core Ruby language. ``` shell make test @@ -80,8 +72,7 @@ make test ### Run Extensive Tests -There are extensive tests in `test/` which cover a wide range of -features of the Ruby core language. +There are extensive tests in `test/` which cover a wide range of features of the Ruby core language. ``` shell make test-all @@ -95,9 +86,7 @@ make test-all TESTS=../test/fiber/test_io.rb ### Run Ruby Spec Suite Tests -The [Ruby Spec Suite](https://github.com/ruby/spec/) is a test suite -that aims to provide an executable description for the behavior of the -language. +The [Ruby Spec Suite](https://github.com/ruby/spec/) is a test suite that aims to provide an executable description for the behaviour of the language. ``` shell make test-spec @@ -110,6 +99,8 @@ Using the address sanitizer is a great way to detect memory issues. ``` shell > ./autogen.sh > mkdir build && cd build -> ../configure cppflags="-fsanitize=address -fno-omit-frame-pointer" optflags=-O1 LDFLAGS="-fsanitize=address -fno-omit-frame-pointer" -> +> ../configure cppflags="-fsanitize=address -fno-omit-frame-pointer" optflags=-O0 LDFLAGS="-fsanitize=address -fno-omit-frame-pointer" +> make ``` + +On Linux it is important to specify -O0 when debugging and this is especially true for ASAN which sometimes works incorrectly at higher optimisation levels. diff --git a/test/fiber/scheduler.rb b/test/fiber/scheduler.rb index 4138015e4b..96b22856d1 100644 --- a/test/fiber/scheduler.rb +++ b/test/fiber/scheduler.rb @@ -30,7 +30,7 @@ class Scheduler @closed = false @lock = Thread::Mutex.new - @blocking = 0 + @blocking = Hash.new.compare_by_identity @ready = [] @urgent = IO.pipe @@ -57,7 +57,7 @@ class Scheduler def run # $stderr.puts [__method__, Fiber.current].inspect - while @readable.any? or @writable.any? or @waiting.any? or @blocking.positive? + while @readable.any? or @writable.any? or @waiting.any? or @blocking.any? # Can only handle file descriptors up to 1024... readable, writable = IO.select(@readable.keys + [@urgent.first], @writable.keys, [], next_timeout) @@ -211,20 +211,22 @@ class Scheduler def block(blocker, timeout = nil) # $stderr.puts [__method__, blocker, timeout].inspect + fiber = Fiber.current + if timeout - @waiting[Fiber.current] = current_time + timeout + @waiting[fiber] = current_time + timeout begin Fiber.yield ensure # Remove from @waiting in the case #unblock was called before the timeout expired: - @waiting.delete(Fiber.current) + @waiting.delete(fiber) end else - @blocking += 1 + @blocking[fiber] = true begin Fiber.yield ensure - @blocking -= 1 + @blocking.delete(fiber) end end end diff --git a/test/fiber/test_scheduler.rb b/test/fiber/test_scheduler.rb index f1030ea3fa..4b1310f0a6 100644 --- a/test/fiber/test_scheduler.rb +++ b/test/fiber/test_scheduler.rb @@ -106,22 +106,24 @@ class TestFiberScheduler < Test::Unit::TestCase end def test_autoload - Object.autoload(:TestFiberSchedulerAutoload, File.expand_path("autoload.rb", __dir__)) + 100.times do + Object.autoload(:TestFiberSchedulerAutoload, File.expand_path("autoload.rb", __dir__)) - thread = Thread.new do - scheduler = Scheduler.new - Fiber.set_scheduler scheduler + thread = Thread.new do + scheduler = Scheduler.new + Fiber.set_scheduler scheduler - 10.times do - Fiber.schedule do - Object.const_get(:TestFiberSchedulerAutoload) + 10.times do + Fiber.schedule do + Object.const_get(:TestFiberSchedulerAutoload) + end end end - end - thread.join - ensure - $LOADED_FEATURES.delete(File.expand_path("autoload.rb", __dir__)) - Object.send(:remove_const, :TestFiberSchedulerAutoload) + thread.join + ensure + $LOADED_FEATURES.delete(File.expand_path("autoload.rb", __dir__)) + Object.send(:remove_const, :TestFiberSchedulerAutoload) + end end end diff --git a/variable.c b/variable.c index f7d9f1c61e..b04d0f6711 100644 --- a/variable.c +++ b/variable.c @@ -2132,7 +2132,7 @@ struct autoload_const { VALUE module; // The name of the constant we are loading. - VALUE name; + ID name; // The value of the constant (after it's loaded). VALUE value; @@ -2308,22 +2308,6 @@ autoload_feature_lookup_or_create(VALUE feature, struct autoload_data **autoload return autoload_data_value; } -#if 0 -static VALUE -autoload_feature_clear_if_empty(VALUE argument) -{ - RUBY_ASSERT_MUTEX_OWNED(autoload_mutex); - - struct autoload_data *autoload_data = (struct autoload_data *)argument; - - if (ccan_list_empty(&autoload_data->constants)) { - rb_hash_delete(autoload_features, autoload_data->feature); - } - - return Qnil; -} -#endif - static struct st_table * autoload_table_lookup_or_create(VALUE module) { @@ -2596,14 +2580,13 @@ autoload_load_needed(VALUE _arguments) struct autoload_load_arguments *arguments = (struct autoload_load_arguments*)_arguments; const char *loading = 0, *src; - struct autoload_data *ele; if (!autoload_defined_p(arguments->module, arguments->name)) { return Qfalse; } - VALUE load = check_autoload_required(arguments->module, arguments->name, &loading); - if (!load) { + VALUE autoload_const_value = check_autoload_required(arguments->module, arguments->name, &loading); + if (!autoload_const_value) { return Qfalse; } @@ -2613,22 +2596,23 @@ autoload_load_needed(VALUE _arguments) } struct autoload_const *autoload_const; - if (!(ele = get_autoload_data(load, &autoload_const))) { + struct autoload_data *autoload_data; + if (!(autoload_data = get_autoload_data(autoload_const_value, &autoload_const))) { return Qfalse; } - if (ele->mutex == Qnil) { - ele->mutex = rb_mutex_new(); - ele->fork_gen = GET_VM()->fork_gen; + if (autoload_data->mutex == Qnil) { + autoload_data->mutex = rb_mutex_new(); + autoload_data->fork_gen = GET_VM()->fork_gen; } - else if (rb_mutex_owned_p(ele->mutex)) { + else if (rb_mutex_owned_p(autoload_data->mutex)) { return Qfalse; } + arguments->mutex = autoload_data->mutex; arguments->autoload_const = autoload_const; - arguments->mutex = ele->mutex; - return load; + return autoload_const_value; } static VALUE @@ -2730,17 +2714,17 @@ rb_autoload_load(VALUE module, ID name) struct autoload_load_arguments arguments = {.module = module, .name = name, .mutex = Qnil, .result = Qnil}; // Figure out whether we can autoload the named constant: - VALUE load = rb_mutex_synchronize(autoload_mutex, autoload_load_needed, (VALUE)&arguments); + VALUE autoload_const_value = rb_mutex_synchronize(autoload_mutex, autoload_load_needed, (VALUE)&arguments); // This confirms whether autoloading is required or not: - if (load == Qfalse) return load; + if (autoload_const_value == Qfalse) return autoload_const_value; arguments.flag = ce->flag & (CONST_DEPRECATED | CONST_VISIBILITY_MASK); // Only one thread will enter here at a time: - return rb_mutex_synchronize(arguments.mutex, autoload_try_load, (VALUE)&arguments); + VALUE result = rb_mutex_synchronize(arguments.mutex, autoload_try_load, (VALUE)&arguments); - // rb_mutex_synchronize(autoload_mutex, autoload_feature_clear_if_empty, (VALUE)&arguments.autoload_data); + return result; } VALUE