Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Autoload is not overwritten by subsequent explicit require #3656

Closed
headius opened this issue Feb 10, 2016 · 6 comments · Fixed by #5764
Closed

Autoload is not overwritten by subsequent explicit require #3656

headius opened this issue Feb 10, 2016 · 6 comments · Fixed by #5764
Labels
core JRuby 9000 ruby 2.3
Milestone
JRuby 9.3.0.0

Comments

@headius
Copy link
Member

headius commented Feb 10, 2016

I'm trying to fix TestAutoload#test_require_implemented_in_ruby_is_called from the MRI suite. The patch I started with is below.

diff --git a/core/src/main/java/org/jruby/runtime/load/LoadService.java b/core/src/main/java/org/jruby/runtime/load/LoadService.java
index a76e457..4b58e6e 100644
--- a/core/src/main/java/org/jruby/runtime/load/LoadService.java
+++ b/core/src/main/java/org/jruby/runtime/load/LoadService.java
@@ -396,7 +396,7 @@ public class LoadService {
     }

     public boolean autoloadRequire(String requireName) {
-        return requireCommon(requireName, false) != RequireState.CIRCULAR;
+        return runtime.getTopSelf().callMethod(runtime.getCurrentContext(), "require", runtime.newString(requireName)).isTrue();
     }

     private enum RequireState {

This matches MRI's behavior and appears to fix the test, except for a little snag...another test starts to fail with a StackOverflowError:

[1/1] TestRequire#test_require_with_loaded_features_pop = 3.09 s
  1) Failure:
TestRequire#test_require_with_loaded_features_pop [/Users/headius/projects/jruby/test/mri/ruby/test_require.rb:680]:
[ruby-core:50645].

1. [1/2] Assertion for "stdout"
   | <[":ok"]> expected but was
   | <["nil", ":ok"]>.

2. [2/2] Assertion for "stderr"
   | <[]> expected but was
   | <["Exception in thread \"Ruby-0-Thread-1: -:1\" java.lang.StackOverflowError",
   |  "\tat java.lang.ClassLoader.findLoadedClass(ClassLoader.java:1033)",
   |  "\tat java.lang.ClassLoader.loadClass(ClassLoader.java:406)",
   |  "\tat java.lang.ClassLoader.loadClass(ClassLoader.java:357)",
   |  "\tat Users.headius.projects.jruby.lib.ruby.stdlib.rubygems.core_ext.kernel_require.RUBY$method$require$0(/Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:52)",
   |  "\tat org.jruby.internal.runtime.methods.CompiledIRMethod.invokeExact(CompiledIRMethod.java:240)",
   |  "\tat org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:110)",
   |  "\tat org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:168)",
   |  "\tat org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:197)",
   |  "\tat org.jruby.RubyClass.finvoke(RubyClass.java:770)",
   |  "\tat org.jruby.runtime.Helpers.invoke(Helpers.java:401)",
   |  "\tat org.jruby.RubyBasicObject.callMethod(RubyBasicObject.java:385)",
   |  "\tat org.jruby.runtime.load.LoadService.autoloadRequire(LoadService.java:399)",
   |  "\tat org.jruby.RubyKernel$1.load(RubyKernel.java:187)",
   |  "\tat org.jruby.RubyModule$Autoload.getConstant(RubyModule.java:4468)",
   |  "\tat org.jruby.RubyModule.getAutoloadConstant(RubyModule.java:4258)",
   |  "\tat org.jruby.RubyModule.getAutoloadConstant(RubyModule.java:4251)",
   |  "\tat org.jruby.RubyModule.resolveUndefConstant(RubyModule.java:3665)",
   |  "\tat org.jruby.RubyModule.getConstantFromNoConstMissing(RubyModule.java:3634)",
   |  "\tat org.jruby.ir.runtime.IRRuntimeHelpers.inheritedSearchConst(IRRuntimeHelpers.java:1147)",
   |  "\tat Users.headius.projects.jruby.lib.ruby.stdlib.rubygems.core_ext.kernel_require.RUBY$method$require$0(/Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:52)",

This is happening inside kernel_require.rb at the point where it first accesses Gem::Specification, and unsurprisingly an autoload in RubyGems' require will redispatch to require and trigger this overflow. However, even though Gem::Specification is set up as an autoload for rubygems/specification, that file is immediately required in rubygems.rb and should overwrite the autoload version.

I do not know why this is happening. If I comment out the autoload, my patch works fine.
@headius
Copy link
Member Author

headius commented Feb 10, 2016

Patch for rubygems.rb that makes this work correctly. It should not be necessary:

diff --git a/lib/ruby/stdlib/rubygems.rb b/lib/ruby/stdlib/rubygems.rb
index f0861b3..fd0b513 100644
--- a/lib/ruby/stdlib/rubygems.rb
+++ b/lib/ruby/stdlib/rubygems.rb
@@ -1222,7 +1222,7 @@ module Gem
   autoload :Source,             'rubygems/source'
   autoload :SourceList,         'rubygems/source_list'
   autoload :SpecFetcher,        'rubygems/spec_fetcher'
-  autoload :Specification,      'rubygems/specification'
+  # autoload :Specification,      'rubygems/specification'
   autoload :Version,            'rubygems/version'

   require "rubygems/specification"

@headius headius added this to the JRuby 9.1.0.0 milestone Mar 3, 2016
@headius
Copy link
Member Author

headius commented Mar 3, 2016

After a bit more research, this is a missing behavior of autoloads for us. When autoloading, the path to be loaded is given to autoload_provided which calls rb_feature_provided which eventually uses their big cache of expanded load path entries and expanded loaded features to figure out if the in-process autoload might already have been fulfilled by a normal require.

Because we do not do this search (and we do not have the structures in place to support it) we end up triggering the autoload again even if it should have been wiped out by a normal require.

@headius headius modified the milestones: JRuby 9.1.1.0, JRuby 9.1.0.0 Apr 20, 2016
@headius
Copy link
Member Author

headius commented Apr 20, 2016

Only reported as a failure in a new 2.3 test, so bumping for now to post 9.1.

@headius headius modified the milestones: JRuby 9.1.1.0, JRuby 9.1.2.0 May 11, 2016
@enebo enebo modified the milestones: JRuby 9.1.2.0, JRuby 9.1.3.0 May 23, 2016
@keeguon
Copy link

keeguon commented May 23, 2016

Just ran into this while trying to generate a config file with warbler on a server which is a bit puzzling because I wasn't able to reproduce it while using rbenv on my local machine on a new Rails app.

@headius headius modified the milestones: JRuby 9.1.4.0, JRuby 9.1.3.0 Aug 22, 2016
@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.1.6.0 Nov 8, 2016
@nirvdrum
Copy link
Contributor

nirvdrum commented Jan 9, 2017

I believe this is the cause of failures I'm seeing with rubber now (by way of fog):

https://travis-ci.org/rubber/rubber/jobs/188061119

Fog interfaces to many different providers. It also has a built-in mocking system so requests don't hit actual cloud providers. Part of the mocking system is a reset action to clear in-memory state. Rather than keep a global list of all loaded providers, it cycles through its known list of providers and sees which ones are set to autoload. Those that aren't are assumed to be loaded and those that are to be autoloaded are skipped since they shouldn't be in use:

https://github.com/fog/fog-core/blob/b34f64027144e4afe2e3092e03596a61480d4bb0/lib/fog/core/mock.rb#L91-L110

Since JRuby isn't clearing the autoload state, the mock cleanup is skipped and bad data lingers around causing test failures for Rubber. This seems to be a regression from JRuby 1.7. I dropped it out of the Rubber test suite because dependencies started going Ruby 2.0+ only, but tests used to pass fine previously.

@headius headius modified the milestones: JRuby 9.2.0.0, JRuby 9.2.1.0 May 15, 2018
@headius headius modified the milestones: JRuby 9.2.1.0, JRuby 9.3.0.0 Oct 26, 2018
This was referenced Nov 19, 2018
Merged
Closed
@headius
Copy link
Member Author

headius commented Jun 18, 2020

This will be fixed once #5764 lands.

@headius headius linked a pull request Jun 18, 2020 that will close this issue
Rework load service, load path, loaded features, and autoload #5764
Merged
This was referenced Jul 2, 2020
Closed
Closed
@headius headius mentioned this issue Sep 22, 2021
Closed
@kares kares mentioned this issue Dec 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core JRuby 9000 ruby 2.3
Projects
None yet
Milestone
JRuby 9.3.0.0
Development

Successfully merging a pull request may close this issue.

Rework load service, load path, loaded features, and autoload jruby/jruby
4 participants
@headius @nirvdrum @enebo @keeguon