[ruby-core:108603] [Ruby master Bug#18782] Race conditions in autoload when loading the same feature with multiple threads.

Issue #18782 has been updated by ioquatix (Samuel Williams).



@fxn said:

> Once the trigger has been executed, it is gone, the way I see it. How is that represented internally, I don't care and should not drive the design of the interface in my view. To me, autoload is not metadata of the module you need to preserve and autoload? should say if there's an autoload not yet triggered in the class/module or ancestors or not.

Maybe, but how it actually works is a lot more nuanced.

Consider multiple threads trying to require a feature which is slow to load. Let's say it's a web sever, and the first thread is cancelled because the request is cancelled. Should this cause all subsequent attempts to use the constant to fail? If you think of it like a trigger, the first thread fires the trigger, fails to load the file, and now it's gone and can never be loaded. But if you look at it as more of a gate or doorway, on transitioning through the doorway, you may eventually get the constant, even if prior threads failed to make it all the way through.

The doorway style is safer because a transient failure won't cause the constant load to fail outright and allows subsequent threads to retry. It's also easier from an implementation point of view to NOT mutate the autoload state while it's being loaded which introduces more complexity in the locking and state management. Essentially I prefer immutable autoload state + read-only operations during load. Deleting the state is a write operation which significantly complicates the internal locking logic.

----------------------------------------
Bug #18782: Race conditions in autoload when loading the same feature with multiple threads.
https://bugs.ruby-lang.org/issues/18782#change-97641

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
I have identified several race conditions in the autoload code.

1. It's possible to race on adding and then deleting items in `autoload_featuremap`. When this happens, two threads will try to load the same file with different autoload data and deadlock.
2. When finishing autoload, it's necessary to clear `ele->state` before setting constants. If this is not synchronised, a thread can see the cleared `ele->state` before seeing the constants and assume the constant is not being autoloaded and then fail with `NameError`.

This test case can reproduce both cases:

```
# test.rb
autoload_path = File.join(__dir__, "foobar.rb")
File.write(autoload_path, 'module Foo; end; module Bar; end')

100_000.times do
	$stderr.puts "--------------------"
	autoload :Foo, autoload_path
	autoload :Bar, autoload_path

	t1 = Thread.new {Foo}
	t2 = Thread.new {Bar}

	t1.join
	t2.join

	Object.send(:remove_const, :Foo)
	Object.send(:remove_const, :Bar)

	$LOADED_FEATURES.delete(autoload_path)
end
```

Example failure of case (1):

```
-------------------- (success)
autoload_by_someone_else ele=0x55f33b806a30 ele->state=(nil)
autoload_by_someone_else ele=0x55f33b806a30 ele->state=(nil)
check_autoload_required 2
autoload_by_someone_else ele=0x55f33b806a30 ele->state=0x7fdd678be780
check_autoload_required 4
autoload_by_someone_else ele=0x55f33b806a30 ele->state=0x7fdd678be780
check_autoload_required 4
ele=0x55f33b806a30 ele->state=0x7fdd678be780 = NULL
check_autoload_required 4
-------------------- (failure)
autoload_by_someone_else ele=0x55f33b806a30 ele->state=(nil)
autoload_by_someone_else ele=0x55f33b6e8f40 ele->state=(nil)
check_autoload_required 2
check_autoload_required 3
autoload_by_someone_else ele=0x55f33b806a30 ele->state=0x7fdd6779d780
check_autoload_required 1
autoload_by_someone_else ele=0x55f33b806a30 ele->state=0x7fdd6779d780
check_autoload_required 1
ele=0x55f33b806a30 ele->state=0x7fdd6779d780 = NULL
ele=0x55f33b6e8f40 ele->state=0x7fdd678be780 = NULL
../test.rb:12:in `join': No live threads left. Deadlock? (fatal)
3 threads, 3 sleeps current:0x000055f33b771250 main thread:0x000055f33b66e090
* #<Thread:0x00007fdd6a2cb0b0 sleep_forever>
   rb_thread_t:0x000055f33b66e090 native:0x00007fdd6a71c3c0 int:0
   
* #<Thread:0x00007fdd676e0090 ../test.rb:9 sleep_forever>
   rb_thread_t:0x000055f33b770ff0 native:0x00007fdd6789e640 int:1 mutex:0x000055f33b7c5100 cond:1
    depended by: tb_thread_id:0x000055f33b66e090
   
* #<Thread:0x00007fdd676e1238 ../test.rb:10 sleep_forever>
   rb_thread_t:0x000055f33b771250 native:0x00007fdd679bf640 int:0
   

	from ../test.rb:12:in `block in <main>'
	from ../test.rb:4:in `times'
	from ../test.rb:4:in `<main>'
make: *** [uncommon.mk:1250: runruby] Error 1
```

Example failure of case (2):

```
[0x7f175fe5b0c8] rb_autoload_str mod=Object id=Foo file="/home/samuel/Projects/ioquatix/ruby/foobar.rb"
[0x7f175fe5b0c8] rb_autoload_str const_set mod=Object id=Foo file="/home/samuel/Projects/ioquatix/ruby/foobar.rb"
[0x7f175fe5b0c8] rb_autoload_str mod=Object id=Bar file="/home/samuel/Projects/ioquatix/ruby/foobar.rb"
[0x7f175fe5b0c8] rb_autoload_str const_set mod=Object id=Bar file="/home/samuel/Projects/ioquatix/ruby/foobar.rb"
[0x7f175fe61d88] rb_const_search_from value == Qundef -> autoloading
[0x7f175fe61e78] rb_const_search_from value == Qundef -> autoloading
[0x7f175fe61e78] Assigning constants...
[0x7f175fe61d88] rb_const_search_from value == Qundef -> autoloading
[0x7f175fe61e78] autoload_const_set name=:Foo value=Foo
[0x7f175fe61e78] autoload_const_set name=:Bar value=Bar
#<Thread:0x00007f175fe61d88 ../test.rb:11 run> terminated with exception (report_on_exception is true):
../test.rb:11:in `block (2 levels) in <main>': uninitialized constant Bar (NameError)
../test.rb:11:in `block (2 levels) in <main>': uninitialized constant Bar (NameError)
make: *** [uncommon.mk:1250: runruby] Error 1
```

These failures are very uncommon but it does impact Ruby as far back as 2.7, and probably earlier.

---Files--------------------------------
0001-Add-RUBY_VM_CRITICAL_SECTION-for-detecting-unexpecte.patch (2.07 KB)
0002-Prevent-race-between-GC-mark-and-autoload-setup.patch (1.01 KB)
0003-Protect-race-on-autoload-state.patch (10.6 KB)
0004-Increase-timeout-for-debug-tests.patch (910 Bytes)
0005-Revert-removal-of-non-conditional-xfree.patch (691 Bytes)


-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>