Remove autoload for constant if the autoload fails

Previously, if an autoload failed (the file was loaded, but the
constant was not defined by the autoloaded file). Ruby will try
to autoload again if you delete the autoloaded file from
$LOADED_FEATURES.  With this change, the autoload and the
constant itself are removed as soon as it fails.

To handle cases where multiple threads are autoloading, when
deleting an autoload, handle the case where another thread
already deleted it.

Fixes [Bug #15790]

spec/ruby/core/module/autoload_spec.rb

@@ -441,6 +441,26 @@ describe "Module#autoload" do
     ScratchPad.recorded.should == [:raise, :raise]
   end
 
+  ruby_version_is "3.1" do
+    it "removes the constant from Module#constants if the loaded file does not define it" do
+      path = fixture(__FILE__, "autoload_o.rb")
+      ScratchPad.record []
+      ModuleSpecs::Autoload.autoload :O, path
+
+      ModuleSpecs::Autoload.const_defined?(:O).should == true
+      ModuleSpecs::Autoload.should have_constant(:O)
+      ModuleSpecs::Autoload.autoload?(:O).should == path
+
+      -> { ModuleSpecs::Autoload::O }.should raise_error(NameError)
+
+      ModuleSpecs::Autoload.const_defined?(:O).should == false
+      ModuleSpecs::Autoload.should_not have_constant(:O)
+      ModuleSpecs::Autoload.autoload?(:O).should == nil
+      -> { ModuleSpecs::Autoload.const_get(:O) }.should raise_error(NameError)
+    end
+  end
+
+  ruby_version_is ""..."3.1" do
     it "does not remove the constant from Module#constants if the loaded file does not define it, but leaves it as 'undefined'" do
       path = fixture(__FILE__, "autoload_o.rb")
       ScratchPad.record []
@@ -452,11 +472,12 @@ describe "Module#autoload" do
 
       -> { ModuleSpecs::Autoload::O }.should raise_error(NameError)
 
-    ModuleSpecs::Autoload.should have_constant(:O)
       ModuleSpecs::Autoload.const_defined?(:O).should == false
+      ModuleSpecs::Autoload.should have_constant(:O)
       ModuleSpecs::Autoload.autoload?(:O).should == nil
       -> { ModuleSpecs::Autoload.const_get(:O) }.should raise_error(NameError)
     end
+  end
 
   it "does not try to load the file again if the loaded file did not define the constant" do
     path = fixture(__FILE__, "autoload_o.rb")
@@ -554,13 +575,35 @@ describe "Module#autoload" do
         # Basically, the parent autoload constant remains in a "undefined" state
         self.autoload?(:DeclaredInParentDefinedInCurrent).should == nil
         const_defined?(:DeclaredInParentDefinedInCurrent).should == false
-        self.should have_constant(:DeclaredInParentDefinedInCurrent)
         -> { DeclaredInParentDefinedInCurrent }.should raise_error(NameError)
 
         ModuleSpecs::Autoload::LexicalScope.send(:remove_const, :DeclaredInParentDefinedInCurrent)
       end
     end
 
+    ruby_version_is "3.1" do
+      it "looks up in parent scope after failed autoload" do
+        @remove << :DeclaredInCurrentDefinedInParent
+        module ModuleSpecs::Autoload
+          ScratchPad.record -> {
+            DeclaredInCurrentDefinedInParent = :declared_in_current_defined_in_parent
+          }
+
+          class LexicalScope
+            autoload :DeclaredInCurrentDefinedInParent, fixture(__FILE__, "autoload_callback.rb")
+            -> { DeclaredInCurrentDefinedInParent }.should_not raise_error(NameError)
+            # Basically, the autoload constant remains in a "undefined" state
+            self.autoload?(:DeclaredInCurrentDefinedInParent).should == nil
+            const_defined?(:DeclaredInCurrentDefinedInParent).should == false
+            -> { const_get(:DeclaredInCurrentDefinedInParent) }.should raise_error(NameError)
+          end
+
+          DeclaredInCurrentDefinedInParent.should == :declared_in_current_defined_in_parent
+        end
+      end
+    end
+
+    ruby_version_is ""..."3.1" do
       it "and fails when finding the undefined autoload constant in the current scope when declared in current and defined in parent" do
         @remove << :DeclaredInCurrentDefinedInParent
         module ModuleSpecs::Autoload
@@ -581,6 +624,7 @@ describe "Module#autoload" do
           DeclaredInCurrentDefinedInParent.should == :declared_in_current_defined_in_parent
         end
       end
+    end
 
     it "in the included modules" do
       @remove << :DefinedInIncludedModule

spec/ruby/core/module/const_set_spec.rb

@@ -101,7 +101,7 @@ describe "Module#const_set" do
       mod.const_get(:Foo).should == 1
     end
 
-    it "does not warn if the previous value was undefined" do
+    it "does not warn after a failed autoload" do
       path = fixture(__FILE__, "autoload_o.rb")
       ScratchPad.record []
       mod = Module.new
@@ -109,7 +109,6 @@ describe "Module#const_set" do
       mod.autoload :Foo, path
       -> { mod::Foo }.should raise_error(NameError)
 
-      mod.should have_constant(:Foo)
       mod.const_defined?(:Foo).should == false
       mod.autoload?(:Foo).should == nil
 

test/ruby/test_autoload.rb

@@ -456,6 +456,31 @@ p Foo::Bar
     end;
   end
 
+  def test_autoload_after_failed_and_removed_from_loaded_features
+    Dir.mktmpdir('autoload') do |tmpdir|
+      autoload_path = File.join(tmpdir, "test-bug-15790.rb")
+      File.write(autoload_path, '')
+
+      assert_separately(%W[-I #{tmpdir}], <<-RUBY)
+        path = #{File.realpath(autoload_path).inspect}
+        autoload :X, path
+        assert_equal(path, Object.autoload?(:X))
+
+        assert_raise(NameError){X}
+        assert_nil(Object.autoload?(:X))
+        assert_equal(false, Object.const_defined?(:X))
+
+        $LOADED_FEATURES.delete(path)
+        assert_equal(false, Object.const_defined?(:X))
+        assert_nil(Object.autoload?(:X))
+
+        assert_raise(NameError){X}
+        assert_equal(false, Object.const_defined?(:X))
+        assert_nil(Object.autoload?(:X))
+      RUBY
+    end
+  end
+
   def add_autoload(path)
     (@autoload_paths ||= []) << path
     ::Object.class_eval {autoload(:AutoloadTest, path)}
@@ -463,7 +488,7 @@ p Foo::Bar
 
   def remove_autoload_constant
     $".replace($" - @autoload_paths)
-    ::Object.class_eval {remove_const(:AutoloadTest)}
+    ::Object.class_eval {remove_const(:AutoloadTest)} if defined? Object::AutoloadTest
     TestAutoload.class_eval {remove_const(:AutoloadTest)} if defined? TestAutoload::AutoloadTest
   end
 end

variable.c

@@ -2237,6 +2237,8 @@ autoload_delete(VALUE mod, ID id)
 	struct autoload_const *ac;
 
 	st_delete(tbl, &n, &load);
+        /* Qfalse can indicate already deleted */
+        if (load != Qfalse) {
             ele = get_autoload_data((VALUE)load, &ac);
             VM_ASSERT(ele);
             if (ele) {
@@ -2255,6 +2257,7 @@ autoload_delete(VALUE mod, ID id)
                 st_delete(RCLASS_IV_TBL(mod), &n, &val);
             }
         }
+    }
 }
 
 static VALUE
@@ -2502,7 +2505,10 @@ rb_autoload_load(VALUE mod, ID id)
     result = rb_ensure(autoload_require, (VALUE)&state,
 		       autoload_reset, (VALUE)&state);
 
-    if (flag > 0 && (ce = rb_const_lookup(mod, id))) {
+    if (!(ce = rb_const_lookup(mod, id)) || ce->value == Qundef) {
+        rb_const_remove(mod, id);
+    }
+    else if (flag > 0) {
         ce->flag |= flag;
     }
     RB_GC_GUARD(load);