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

Possible autoload issue #2874

Closed
headius opened this issue Apr 24, 2015 · 4 comments
Closed

Possible autoload issue #2874

headius opened this issue Apr 24, 2015 · 4 comments

Comments

@headius
Copy link
Member

headius commented Apr 24, 2015

The script shown in this bug comment fails on JRuby: rubinius/rubinius#2934 (comment)

Failure:

NameError: uninitialized constant RSpec::Matchers::BuiltIn
  const_missing at org/jruby/RubyModule.java:2726
         (root) at blah.rb:7

Rubinius fails in a similar way. Could be an autoload threading problem. Investigating.

@headius
Copy link
Member Author

headius commented Apr 27, 2015

So far this does not appear to be related to autoload, and more likely related to threading. If I capture a heap dump at the point where this constant is found missing, I can see that indeed there is no BuiltIn constant defined in Matchers. However the logic for BuiltIn is loaded as a normal module. It's a module that itself defines several autoloads, but that should not be coming into play here.

I also confirmed that theory by defining BuiltIn.autoload to just do the require immediately, thereby skipping any lazy loading logic. The issue remains.

I suspect that multi-threaded access of a class's constants has a bug, perhaps two threads attempting to initialize the same class's constant table are creating two different constant maps.

@headius
Copy link
Member Author

headius commented Apr 27, 2015

Ah-ha, I was wrong! Autoload is involved, but not at the BuiltIn level...the RSpec::Matchers constant is "autoloaded", albeit not the usual way (which is why I couldn't see it):

  MODULES_TO_AUTOLOAD = {
    :Matchers     => "rspec/expectations",
    :Expectations => "rspec/expectations",
    :Mocks        => "rspec/mocks"
  }

  def self.const_missing(name)
    # Load rspec-expectations when RSpec::Matchers is referenced. This allows
    # people to define custom matchers (using `RSpec::Matchers.define`) before
    # rspec-core has loaded rspec-expectations (since it delays the loading of
    # it to allow users to configure a different assertion/expectation
    # framework). `autoload` can't be used since it works with ruby's built-in
    # require (e.g. for files that are available relative to a load path dir),
    # but not with rubygems' extended require.
    #
    # As of rspec 2.14.1, we no longer require `rspec/mocks` and
    # `rspec/expectations` when `rspec` is required, so we want
    # to make them available as an autoload. For more info, see:
    require MODULES_TO_AUTOLOAD.fetch(name) { return super }
    ::RSpec.const_get(name)
  end

This logic is not thread-safe, but I have yet to determine if it's JRuby's fault or RSpecs.

@headius
Copy link
Member Author

headius commented Apr 27, 2015

Ahh yes, now I understand. This is an RSpec bug. /cc @myronmarston @samphippen

This logic is essentially the same problem as using defined?(SomeConst) to determine whether a library has been loaded. Here's a possible sequence leading to the error above.

  • Thread A begins accessing RSpec::Matchers
  • A can't find the constant, so the const_missing triggers
  • A starts loading matchers.rb, and right away Matchers is defined (but still empty)
  • Thread B begins accessing RSpec::Matchers
  • B sees that the constant is there and proceeds to look up BuiltIn, skipping the const_missing altogether
  • B has an incomplete view of Matchers since it is still being loaded, and as a result BuiltIn (or any number of other constants and methods) is missing.

Synchronizing const_missing does not help, because once the Matchers module is defined we won't even call it. Autoload is not at fault here obviously, but neither is require logic, since no amount of locking can prevent this code from possibly seeing a partially-initialized Matchers module. RSpec needs to change how it manages these constants.

And finally, a detail I should have checked immediately...this happens in MRI too:

[] ~/projects/jruby-1.7 $ rvm ruby-2.2 do ruby blah.rb
blah.rb:11:in `block (2 levels) in <main>': uninitialized constant RSpec::Matchers::BuiltIn (NameError)

@headius
Copy link
Member Author

headius commented Apr 27, 2015

Filed rspec/rspec-dev#121.

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

No branches or pull requests

1 participant