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.

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

Autoload broken by path canonicalization #2788

Closed
headius opened this issue Mar 31, 2015 · 1 comment
Closed

Autoload broken by path canonicalization #2788

headius opened this issue Mar 31, 2015 · 1 comment

Comments

@headius
Copy link
Member

headius commented Mar 31, 2015

We have broken autoload in recent weeks or months. I have tracked the problem down to autoload logic not handling RubyGems canonicalization of file paths and our LoadService's failure to recognize that the canonicalized path and the absolute path refer to the same file.

An example of how this breaks:

Main script:

module Foo
  autoload :Bar, 'autoloaded.rb'
end
require_relative 'autoloaded.rb'

autoloaded.rb:

module Foo
  class Bar
    undef_method :==
  end
end

The resulting stack trace:

[] ~/projects/jruby $ jruby script.rb
NameError: Undefined method == for class 'Foo::Bar'
  undef_method at org/jruby/RubyModule.java:2607
   <class:Bar> at /Users/headius/projects/jruby/autoloaded.rb:3
  <module:Foo> at /Users/headius/projects/jruby/autoloaded.rb:2
         <top> at /Users/headius/projects/jruby/autoloaded.rb:1
       require at org/jruby/RubyKernel.java:966
        (root) at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:1
       require at /Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54
         <top> at script.rb:4

It looks like something's awry with undef_method, but what's actually happening is that the autoloaded.rb script is executing twice: once when required directly and once when the first access of Foo::Bar in autoloaded.rb (to open or reopen the Bar class) triggers the autoload.

Our autoload logic detects an in-progress autoload by looking for a circular require. Circular requires are detected by looking in a map of require locks to see if the current thread has already locked this file.

However, if the file is required via an absolute path, the entry in the table does not match and it proceeds to attempt to load the relative path. That execution of undef_method happens first and succeeds, it exits the script body, and then the first load of the autoloaded script proceeds to undef_method a method that isn't there anymore.

Very sneaky, because the error happens without any evidence of the second load in the stack...but I saw this in the context of a rails app with numerous other double-load entries in the backtrace.

This also breaks circular require detection in general, but the autoload scenario is more of an issue because it is so heavily used by Ruby frameworks.

Must be fixed for pre2. This may require another overhaul of LoadService and its management of required and found paths.

@headius headius added this to the 9.0.0.0.pre2 milestone Mar 31, 2015
headius added a commit that referenced this issue Apr 1, 2015
@headius
Copy link
Member Author

headius commented Apr 2, 2015

This should be fixed...but we need a better rework of require + loaded features + load path that matches MRI a bit better (and which avoids us doing as many manual searches and canonicalizations as we do now). I will file a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant