[ruby-core:108558] [Ruby master Bug#15790] Strange interaction between autoload and $LOADED_FEATURES

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


The behaviour described here does not seem strange to me, at least, I'm not sure if it's problematic. @fxn what was the actual issue at the root of the behaviour described here?

Even more challenging is how this PR is implemented, given that there is no locking around `rb_const_remove` it seems to introduce yet another potential race condition between threads.

Finally, it's also not clear that the proposed PR actually addresses all permutations of the problem.

There is another spec here:

```ruby
    it "raises a LoadError in each thread if the file does not exist" do
      file = fixture(__FILE__, "autoload_does_not_exist.rb")
      start = false

      threads = Array.new(10) do
        Thread.new do
          Thread.pass until start
          begin
            ModuleSpecs::Autoload.autoload :FileDoesNotExist, file
            Thread.pass
            ModuleSpecs::Autoload::FileDoesNotExist
          rescue LoadError => e
            e
          ensure
            Thread.pass
          end
        end
      end

      start = true
      threads.each { |t|
        t.value.should be_an_instance_of(LoadError)
      }
    end
```

This spec requires all 10 threads to raise a LoadError. With the current implementation, this leaves the autoload state in place.

We can see the unusual behaviour manifest:

```
autoload :X, "non_existant.rb"

begin
  X
rescue LoadError => error
  pp error
  # #<LoadError: cannot load such file -- non_existant.rb>
end

pp autoload? :X
# "non_existant.rb"

autoload :Y, "./empty.rb"

begin
  Y
rescue NameError => error
  pp error
  # #<NameError: uninitialized constant Y>
end

pp autoload? :Y
# nil
```

We now have specs which are effectively at odds with each other, and only differ in very nuanced details which probably shouldn't matter.

I'm not sure whether we either (1) revert the behaviour/PR here or (2) handle all cases where the required feature fails to load.

I think (1) is easier and more consistent. Autoload is state which is separate from the constant. It's a way of loading the constant if it's not loaded. The implementation of (2) is far more complex and tricky to get right. Knowing when to remove the autoload state does not seem trivial to me. There are edge cases which we have to consider like if the thread is terminated during the require (but would have otherwise succeeded). I'm not even sure we can enumerate all these conditions... Should this prevent another thread from successfully requiring the file? It also seems like it's going to be easier to revert this PR than it is to change a long established spec.


----------------------------------------
Bug #15790: Strange interaction between autoload and $LOADED_FEATURES
https://bugs.ruby-lang.org/issues/15790#change-97597

* Author: fxn (Xavier Noria)
* Status: Closed
* Priority: Normal
* ruby -v: ruby 2.6.1p33 (2019-01-30 revision 66950) [x86_64-darwin18]
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
If an autoload fails and we remove its associated file from `$LOADED_FEATURES`, the autoload is back:

```
$ cat x.rb
Y = 1 # should be X, emulates a typo

$ cat test.rb
def au
  Object.autoload?(:X).inspect
end

x_rb = File.realpath("x.rb")
autoload :X, x_rb

puts "before failed autoload autoload path is #{au}"

X rescue nil

puts "after failed autoload autoload path is #{au}"

$LOADED_FEATURES.delete(x_rb)

puts "after $LOADED_FEATURES deletion autoload path is #{au}"
```

The output is

```
$ ruby -v test.rb
ruby 2.6.1p33 (2019-01-30 revision 66950) [x86_64-darwin18]
before failed autoload autoload path is "/Users/fxn/tmp/x.rb"
after failed autoload autoload path is nil
after $LOADED_FEATURES deletion autoload path is "/Users/fxn/tmp/x.rb"
```

See? Last line would be expected to print a `nil` autoload path.



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

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