summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@ruby-lang.org>2023-03-21 15:32:44 +0900
committernagachika <nagachika@ruby-lang.org>2023-03-21 15:32:44 +0900
commit26b3f0b6a9489860e1d312369d784495b080daa3 (patch)
treef29019fd31aa6947db4318a2b2e9d2c599c81729
parent485e0e46a58ff1a28eecd63b804b3dbea4b95b98 (diff)
merge revision(s) 8ce2fb9bbbaea14737c84385b1573f743a30f773,3a0f6ce1d31eefd8af01b50f3632a64d64e8f8c1: [Backport #19415]
Only emit circular dependency warning for owned thread shields [Bug #19415] If multiple threads attemps to load the same file concurrently it's not a circular dependency issue. So we check that the existing ThreadShield is owner by the current fiber before warning about circular dependencies. --- internal/thread.h | 1 + load.c | 3 ++- spec/ruby/core/kernel/shared/require.rb | 11 +++++++++++ spec/ruby/fixtures/code/concurrent_require_fixture.rb | 4 ++++ test/ruby/test_require.rb | 3 --- thread.c | 11 +++++++++++ 6 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 spec/ruby/fixtures/code/concurrent_require_fixture.rb Use Thread.pass until thread.stop? to wait for thread to block [Bug #19415] It should be more reliable --- spec/ruby/fixtures/code/concurrent_require_fixture.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
-rw-r--r--internal/thread.h1
-rw-r--r--load.c2
-rw-r--r--spec/ruby/core/kernel/shared/require.rb11
-rw-r--r--spec/ruby/fixtures/code/concurrent_require_fixture.rb4
-rw-r--r--test/ruby/test_require.rb3
-rw-r--r--thread.c11
-rw-r--r--version.h2
7 files changed, 29 insertions, 5 deletions
diff --git a/internal/thread.h b/internal/thread.h
index 919ad96580..c7fe16eb6b 100644
--- a/internal/thread.h
+++ b/internal/thread.h
@@ -28,6 +28,7 @@ VALUE rb_get_coverages(void);
int rb_get_coverage_mode(void);
VALUE rb_default_coverage(int);
VALUE rb_thread_shield_new(void);
+bool rb_thread_shield_owned(VALUE self);
VALUE rb_thread_shield_wait(VALUE self);
VALUE rb_thread_shield_release(VALUE self);
VALUE rb_thread_shield_destroy(VALUE self);
diff --git a/load.c b/load.c
index bc0571124d..498c66a3fe 100644
--- a/load.c
+++ b/load.c
@@ -821,7 +821,7 @@ load_lock(rb_vm_t *vm, const char *ftptr, bool warn)
(*init)();
return (char *)"";
}
- if (warn) {
+ if (warn && rb_thread_shield_owned((VALUE)data)) {
VALUE warning = rb_warning_string("loading in progress, circular require considered harmful - %s", ftptr);
rb_backtrace_each(rb_str_append, warning);
rb_warning("%"PRIsVALUE, warning);
diff --git a/spec/ruby/core/kernel/shared/require.rb b/spec/ruby/core/kernel/shared/require.rb
index 9a2c38be1a..f4645b0242 100644
--- a/spec/ruby/core/kernel/shared/require.rb
+++ b/spec/ruby/core/kernel/shared/require.rb
@@ -237,6 +237,17 @@ describe :kernel_require, shared: true do
}.should complain(/circular require considered harmful/, verbose: true)
ScratchPad.recorded.should == [:loaded]
end
+
+ ruby_bug "#17340", ''...'3.3' do
+ it "loads a file concurrently" do
+ path = File.expand_path "concurrent_require_fixture.rb", CODE_LOADING_DIR
+ ScratchPad.record(@object)
+ -> {
+ @object.require(path)
+ }.should_not complain(/circular require considered harmful/, verbose: true)
+ ScratchPad.recorded.join
+ end
+ end
end
describe "(non-extensioned path)" do
diff --git a/spec/ruby/fixtures/code/concurrent_require_fixture.rb b/spec/ruby/fixtures/code/concurrent_require_fixture.rb
new file mode 100644
index 0000000000..d4ce734183
--- /dev/null
+++ b/spec/ruby/fixtures/code/concurrent_require_fixture.rb
@@ -0,0 +1,4 @@
+object = ScratchPad.recorded
+thread = Thread.new { object.require(__FILE__) }
+Thread.pass until thread.stop?
+ScratchPad.record(thread)
diff --git a/test/ruby/test_require.rb b/test/ruby/test_require.rb
index 77cdae64d9..f53bd2ce5f 100644
--- a/test/ruby/test_require.rb
+++ b/test/ruby/test_require.rb
@@ -562,9 +562,6 @@ class TestRequire < Test::Unit::TestCase
assert_equal(true, (t1_res ^ t2_res), bug5754 + " t1:#{t1_res} t2:#{t2_res}")
assert_equal([:pre, :post], scratch, bug5754)
-
- assert_match(/circular require/, output)
- assert_match(/in #{__method__}'$/o, output)
}
ensure
$VERBOSE = verbose
diff --git a/thread.c b/thread.c
index de29340713..b1b5dd0f53 100644
--- a/thread.c
+++ b/thread.c
@@ -5040,6 +5040,17 @@ rb_thread_shield_new(void)
return thread_shield;
}
+bool
+rb_thread_shield_owned(VALUE self)
+{
+ VALUE mutex = GetThreadShieldPtr(self);
+ if (!mutex) return false;
+
+ rb_mutex_t *m = mutex_ptr(mutex);
+
+ return m->fiber == GET_EC()->fiber_ptr;
+}
+
/*
* Wait a thread shield.
*
diff --git a/version.h b/version.h
index b5bbcf46d5..d72eff87f2 100644
--- a/version.h
+++ b/version.h
@@ -11,7 +11,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 4
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 204
+#define RUBY_PATCHLEVEL 205
#define RUBY_RELEASE_YEAR 2023
#define RUBY_RELEASE_MONTH 3