Release the autoload claim when Autoload#load has nothing to load

core/src/main/java/org/jruby/RubyModule.java

@@ -3225,7 +3225,7 @@
         if (hash.isBuiltin(this)) return id;
 
         // we truncate for Java hashcode
         return (int) hash.call(context, this, this).convertToInteger().getLongValue();
     }
 
     @JRubyMethod(name = "hash")
@@ -3234,8 +3234,8 @@
     }
 
     @Override
     public RubyString to_s() {
         return (RubyString) to_s(getCurrentContext());
     }
 
     /** rb_mod_to_s
@@ -6689,11 +6689,14 @@
                 // This method needs to be synchronized for removing Autoload
                 // from autoloadMap when it's loaded.
                 LoadService loadService = loadService(context);
-                if (!loadService.featureAlreadyLoaded(path.asJavaString())) {
-                    if (loadService.autoloadRequire(path)) {
-                        // Do not finish autoloading by cyclic autoload
-                        finishAutoload(context, symbol);
-                    }
+                if (loadService.featureAlreadyLoaded(path.asJavaString())) {
+                    // Nothing to load here: the feature is loaded, or a direct require of it is in
+                    // progress. Keeping the claim would make that require's definition of the
+                    // constant look like this autoload's own and leave UNDEF in the constant table.
+                    this.ctx = null;
+                } else if (loadService.autoloadRequire(path)) {
+                    // Do not finish autoloading by cyclic autoload
+                    finishAutoload(context, symbol);
                 }
             } catch (LoadError | RuntimeError lre) {
                 // reset ctx to null for a future attempt to load

spec/ruby/core/kernel/autoload_spec.rb

@@ -164,6 +164,39 @@ def go
       KernelSpecs::AutoloadMethod2::AutoloadFromIncludedModule2.loaded.should == :autoload_from_included_module2
     end
   end
+
+  describe "after the autoload was satisfied by requiring the file directly" do
+    before :each do
+      @path = fixture(__FILE__, "autoload_satisfied_by_require.rb")
+      Kernel.autoload :KSAutoloadSatisfiedByRequire, @path
+      ScratchPad.record []
+    end
+
+    after :each do
+      if Object.const_defined?(:KSAutoloadSatisfiedByRequire, false)
+        Object.send(:remove_const, :KSAutoloadSatisfiedByRequire)
+      end
+    end
+
+    it "defines a regular constant that no longer depends on $LOADED_FEATURES" do
+      require @path
+      ScratchPad.recorded.should == [:loaded]
+      Kernel.autoload?(:KSAutoloadSatisfiedByRequire).should be_nil
+      klass = KSAutoloadSatisfiedByRequire
+      data = Marshal.dump(klass.new)
+
+      $LOADED_FEATURES.replace(@loaded_features)
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+
+    it "returns the constant from remove_const" do
+      require @path
+      klass = KSAutoloadSatisfiedByRequire
+
+      Object.send(:remove_const, :KSAutoloadSatisfiedByRequire).should equal(klass)
+    end
+  end
 end
 
 describe "Kernel.autoload?" do

spec/ruby/core/kernel/fixtures/autoload_satisfied_by_require.rb

@@ -0,0 +1,4 @@
+ScratchPad << :loaded
+
+class KSAutoloadSatisfiedByRequire
+end

spec/ruby/core/module/autoload_spec.rb

@@ -320,6 +320,138 @@ module ModuleSpecs::Autoload
     end
   end
 
+  describe "after the autoload was satisfied by requiring the file directly" do
+    before :each do
+      @path = fixture(__FILE__, "autoload_satisfied_by_require.rb")
+      ModuleSpecs::Autoload.autoload :SatisfiedByRequire, @path
+      @remove << :SatisfiedByRequire
+      ScratchPad.record []
+    end
+
+    it "defines a regular constant that no longer depends on $LOADED_FEATURES" do
+      require @path
+      ScratchPad.recorded.should == [:loaded]
+      ModuleSpecs::Autoload.autoload?(:SatisfiedByRequire).should be_nil
+      klass = ModuleSpecs::Autoload::SatisfiedByRequire
+      data = Marshal.dump(klass.new)
+
+      $LOADED_FEATURES.replace(@loaded_features)
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+
+    it "defines a regular constant that no longer depends on $LOAD_PATH" do
+      ModuleSpecs::Autoload.autoload :SatisfiedByRequire, "autoload_satisfied_by_require"
+      $:.push File.dirname(@path)
+      begin
+        require "autoload_satisfied_by_require.rb"
+      ensure
+        $:.pop
+      end
+      ScratchPad.recorded.should == [:loaded]
+      klass = ModuleSpecs::Autoload::SatisfiedByRequire
+      data = Marshal.dump(klass.new)
+
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+
+    it "returns the constant from remove_const" do
+      require @path
+      klass = ModuleSpecs::Autoload::SatisfiedByRequire
+      @remove.delete(:SatisfiedByRequire)
+
+      ModuleSpecs::Autoload.send(:remove_const, :SatisfiedByRequire).should equal(klass)
+      ModuleSpecs::Autoload.const_defined?(:SatisfiedByRequire).should be_false
+    end
+
+    it "can be registered as an autoload again after remove_const" do
+      require @path
+      klass = ModuleSpecs::Autoload::SatisfiedByRequire
+      ModuleSpecs::Autoload.send(:remove_const, :SatisfiedByRequire)
+      $LOADED_FEATURES.replace(@loaded_features)
+
+      ModuleSpecs::Autoload.autoload :SatisfiedByRequire, @path
+      ModuleSpecs::Autoload.autoload?(:SatisfiedByRequire).should == @path
+      ModuleSpecs::Autoload::SatisfiedByRequire.should_not equal(klass)
+      ScratchPad.recorded.should == [:loaded, :loaded]
+    end
+
+    it "keeps the constant defined by the file when the require raises afterwards" do
+      path = fixture(__FILE__, "autoload_satisfied_by_require_raise.rb")
+      ModuleSpecs::Autoload.autoload :SatisfiedByRequireRaise, path
+      @remove << :SatisfiedByRequireRaise
+
+      -> { require path }.should raise_error(RuntimeError, "raised after defining the constant")
+      ScratchPad.recorded.should == [:loaded]
+      ModuleSpecs::Autoload.autoload?(:SatisfiedByRequireRaise).should be_nil
+      klass = ModuleSpecs::Autoload::SatisfiedByRequireRaise
+      data = Marshal.dump(klass.new)
+
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+
+    it "applies to an autoload the required file declares for a nested constant" do
+      nested = fixture(__FILE__, "autoload_satisfied_by_require_nested.rb")
+      inner = fixture(__FILE__, "autoload_satisfied_by_require_nested_inner.rb")
+      ModuleSpecs::Autoload.autoload :SatisfiedByRequireNested, nested
+      @remove << :SatisfiedByRequireNested
+
+      require nested
+      ModuleSpecs::Autoload::SatisfiedByRequireNested.autoload?(:Inner).should == inner
+      require inner
+      ModuleSpecs::Autoload::SatisfiedByRequireNested.autoload?(:Inner).should be_nil
+      klass = ModuleSpecs::Autoload::SatisfiedByRequireNested::Inner
+      data = Marshal.dump(klass.new)
+
+      $LOADED_FEATURES.replace(@loaded_features)
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:nested_loaded, :inner_loaded]
+    end
+
+    it "applies when the file was loaded with require_relative" do
+      require fixture(__FILE__, "autoload_satisfied_by_require_relative.rb")
+      ScratchPad.recorded.should == [:loaded]
+      klass = ModuleSpecs::Autoload::SatisfiedByRequire
+      data = Marshal.dump(klass.new)
+
+      $LOADED_FEATURES.replace(@loaded_features)
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+  end
+
+  describe "when one file defines several autoloaded constants" do
+    before :each do
+      @path = fixture(__FILE__, "autoload_siblings.rb")
+      ModuleSpecs::Autoload.autoload :SiblingA, @path
+      ModuleSpecs::Autoload.autoload :SiblingB, @path
+      @remove << :SiblingA << :SiblingB
+      ScratchPad.record []
+    end
+
+    it "defines every constant the file declares as a regular constant" do
+      ModuleSpecs::Autoload::SiblingA
+      ScratchPad.recorded.should == [:loaded]
+      ModuleSpecs::Autoload.autoload?(:SiblingB).should be_nil
+      klass = ModuleSpecs::Autoload::SiblingB
+      data = Marshal.dump(klass.new)
+
+      $LOADED_FEATURES.replace(@loaded_features)
+      Thread.new { Marshal.load(data).class }.value.should equal(klass)
+      ScratchPad.recorded.should == [:loaded]
+    end
+
+    it "returns the sibling constant from remove_const" do
+      ModuleSpecs::Autoload::SiblingA
+      klass = ModuleSpecs::Autoload::SiblingB
+      @remove.delete(:SiblingB)
+
+      ModuleSpecs::Autoload.send(:remove_const, :SiblingB).should equal(klass)
+    end
+  end
+
   describe "after the autoload is triggered by require" do
     before :each do
       @path = tmp("autoload.rb")

spec/ruby/core/module/fixtures/autoload_satisfied_by_require.rb

@@ -0,0 +1,6 @@
+ScratchPad << :loaded
+
+module ModuleSpecs::Autoload
+  class SatisfiedByRequire
+  end
+end

spec/ruby/core/module/fixtures/autoload_satisfied_by_require_nested.rb

@@ -0,0 +1,7 @@
+ScratchPad << :nested_loaded
+
+module ModuleSpecs::Autoload
+  class SatisfiedByRequireNested
+    autoload :Inner, File.expand_path("autoload_satisfied_by_require_nested_inner.rb", __dir__)
+  end
+end

spec/ruby/core/module/fixtures/autoload_satisfied_by_require_nested_inner.rb

@@ -0,0 +1,8 @@
+ScratchPad << :inner_loaded
+
+module ModuleSpecs::Autoload
+  class SatisfiedByRequireNested
+    class Inner
+    end
+  end
+end

spec/ruby/core/module/fixtures/autoload_satisfied_by_require_raise.rb

@@ -0,0 +1,8 @@
+ScratchPad << :loaded
+
+module ModuleSpecs::Autoload
+  class SatisfiedByRequireRaise
+  end
+end
+
+raise "raised after defining the constant"

spec/ruby/core/module/fixtures/autoload_satisfied_by_require_relative.rb

@@ -0,0 +1 @@
+require_relative "autoload_satisfied_by_require"

spec/ruby/core/module/fixtures/autoload_siblings.rb

@@ -0,0 +1,9 @@
+ScratchPad << :loaded
+
+module ModuleSpecs::Autoload
+  class SiblingA
+  end
+
+  class SiblingB
+  end
+end

test/jruby/autoload_required/assigned.rb

@@ -0,0 +1,3 @@
+$autoload_required_loads << __FILE__
+
+TestAutoload::Assigned = Class.new

test/jruby/autoload_required/circular_first.rb

@@ -0,0 +1,9 @@
+$autoload_required_loads << __FILE__
+require File.expand_path("circular_second", __dir__)
+
+class TestAutoload
+  module Circular
+    class First
+    end
+  end
+end

test/jruby/autoload_required/circular_second.rb

@@ -0,0 +1,9 @@
+$autoload_required_loads << __FILE__
+require File.expand_path("circular_first", __dir__)
+
+class TestAutoload
+  module Circular
+    class Second
+    end
+  end
+end

test/jruby/autoload_required/colon2.rb

@@ -0,0 +1,4 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload::Colon2
+end

test/jruby/autoload_required/concurrent.rb

@@ -0,0 +1,6 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  class Concurrent
+  end
+end

test/jruby/autoload_required/eager/alpha.rb

@@ -0,0 +1,8 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  module Eager
+    class Alpha
+    end
+  end
+end

test/jruby/autoload_required/eager/beta.rb

@@ -0,0 +1,8 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  module Eager
+    class Beta < Alpha
+    end
+  end
+end

test/jruby/autoload_required/eager/gamma.rb

@@ -0,0 +1,8 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  module Eager
+    module Gamma
+    end
+  end
+end

test/jruby/autoload_required/java_import.rb

@@ -0,0 +1,7 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  module Imported
+    java_import java.util.ArrayList
+  end
+end

test/jruby/autoload_required/loaded.rb

@@ -0,0 +1,6 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  class Loaded
+  end
+end

test/jruby/autoload_required/pair.rb

@@ -0,0 +1,11 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  module Siblings
+    class PairA
+    end
+
+    class PairB
+    end
+  end
+end

test/jruby/autoload_required/private.rb

@@ -0,0 +1,6 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  class Private
+  end
+end

test/jruby/autoload_required/table_slot.rb

@@ -0,0 +1,6 @@
+$autoload_required_loads << __FILE__
+
+class TestAutoload
+  class TableSlot
+  end
+end