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(-)
This commit is contained in:
nagachika 2023-03-21 15:32:44 +09:00
parent 485e0e46a5
commit 26b3f0b6a9
7 changed files with 29 additions and 5 deletions

View file

@ -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);

2
load.c
View file

@ -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);

View file

@ -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

View file

@ -0,0 +1,4 @@
object = ScratchPad.recorded
thread = Thread.new { object.require(__FILE__) }
Thread.pass until thread.stop?
ScratchPad.record(thread)

View file

@ -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

View file

@ -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.
*

View file

@ -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