Project

General

Profile

Bug #12999

there still exist race conditions in require

Added by shugo (Shugo Maeda) over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:78464]

Description

TestRequire#test_require_with_loaded_features_pop fails by increasing the number of threads,
so it seems that there still exist race conditions in require.

lexington:ruby$ cat t/require_test.rb
require "tempfile"

Tempfile.create(%w'bug-7530- .rb') {|script|
  script.close
  PATH = script.path
  THREADS = 10
  ITERATIONS_PER_THREAD = 1000

  THREADS.times.map {
    Thread.new do
      ITERATIONS_PER_THREAD.times do
        require PATH
        $".delete_if {|p| Regexp.new(PATH) =~ p}
      end
    end
  }.each(&:join)
  p :ok
}
lexington:ruby$ ./ruby t/require_test.rb
/home/shugo/local/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require': destroyed thread shield - 0x00557a99ff5a78 (ThreadError)
    from /home/shugo/local/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from t/require_test.rb:12:in `block (4 levels) in <main>'
    from t/require_test.rb:11:in `times'
    from t/require_test.rb:11:in `block (3 levels) in <main>'
lexington:ruby$ ./ruby t/require_test.rb
/home/shugo/local/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require': Attempt to unlock a mutex which is locked by another thread (ThreadError)
    from /home/shugo/local/lib/ruby/2.4.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from t/require_test.rb:12:in `block (4 levels) in <main>'
    from t/require_test.rb:11:in `times'
    from t/require_test.rb:11:in `block (3 levels) in <main>'

Related issues

Related to Ruby master - Bug #7530: Concurrent loads fail with mutex errorsClosedActions

Associated revisions

Revision 11f9b8c0
Added by shugo (Shugo Maeda) over 2 years ago

Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@56985 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 56985
Added by shugo (Shugo Maeda) over 2 years ago

Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

Revision 56985
Added by shugo (Shugo Maeda) over 2 years ago

Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

Revision 56985
Added by shugo (Shugo Maeda) over 2 years ago

Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

Revision 56985
Added by shugo (Shugo Maeda) over 2 years ago

Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

History

#1

Updated by shugo (Shugo Maeda) over 2 years ago

  • Related to Bug #7530: Concurrent loads fail with mutex errors added

Updated by shugo (Shugo Maeda) over 2 years ago

In r56965, load_lock() was changed as follows:

    switch (rb_thread_shield_wait((VALUE)data)) {
      case Qfalse:
        data = (st_data_t)ftptr;
        st_insert(loading_tbl, data, (st_data_t)rb_thread_shield_new());
        return 0;
      case Qnil:
        return 0;
    }

However, if the thread shield is destroyed, it's not necessary to insert
a new entry to the loading table, is it?

With the following fix, ThreadError doesn't seem to occur:

diff --git a/load.c b/load.c
index 3cf9ce0..c37f8bc 100644
--- a/load.c
+++ b/load.c
@@ -745,9 +745,6 @@ load_lock(const char *ftptr)
     }
     switch (rb_thread_shield_wait((VALUE)data)) {
       case Qfalse:
-   data = (st_data_t)ftptr;
-   st_insert(loading_tbl, data, (st_data_t)rb_thread_shield_new());
-   return 0;
       case Qnil:
    return 0;
     }
@@ -759,7 +756,10 @@ release_thread_shield(st_data_t *key, st_data_t *value, st_data_t done, int exis
 {
     VALUE thread_shield = (VALUE)*value;
     if (!existing) return ST_STOP;
-    if (done ? rb_thread_shield_destroy(thread_shield) : rb_thread_shield_release(thread_shield)) {
+    if (done) {
+   rb_thread_shield_destroy(thread_shield);
+    }
+    else if (rb_thread_shield_release(thread_shield)) {
    /* still in-use */
    return ST_CONTINUE;
     }
#3

Updated by shugo (Shugo Maeda) over 2 years ago

  • Status changed from Open to Closed

Applied in changeset r56985.


Don't insert an entry to loading_tbl if another thread succeed to load.

If rb_thread_shield_wait() returns Qfalse, the file has been successfully
loaded by another thread, so there is no need to insert a new entry into
loading_tbl. [ruby-core:78464] [Bug #12999]

Also available in: Atom PDF