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

Self requiring file breaks inheritance hooks #3032

Closed
Xanthus opened this issue Jun 10, 2015 · 6 comments
Closed

Self requiring file breaks inheritance hooks #3032

Xanthus opened this issue Jun 10, 2015 · 6 comments

Comments

@Xanthus
Copy link

Xanthus commented Jun 10, 2015

So first of all, this only happens when you do something lazy like I did and let a file require itself by iterating through a directory and requiring all files. The only reason I'm posting it is because the issue doesn't happen in MRI (tested on 2.2.1) but does in JRuby (1.7.19)

Basically in this contrived example you're supposed to end up with an inheritance tree like this:

BaseClass
--->MiddleClass
------->Subclass1
------->Subclass2

MiddleClass has an inherited hook that registers its subclasses in a class instance variable. The file containing BaseClass loads the file containing MiddleClass. That then loads Subclass1, itself and then Subclass2. The result is that only Subclass2 gets registered by the inheritance hook.

This is a real edge case but I thought it would be worth reporting. I created a gist with a script to reproduce this by creating the files: https://gist.github.com/Xanthus/9494347f4345798113d9#file-generate_self_requiring_files-rb

run "ruby generate_self_requiring_files.rb"
then run "ruby base_class.rb"

You should see output showing failure or success.

@enebo
Copy link
Member

enebo commented Jun 10, 2015

This might be fixed on 1.7.20 already. This is because you are requiring the same file twice and it is getting registered twice in LOAD_PATH so first subclass probbably happens before it reloads the the mddleclass for a second time, then loads second subclass which then registers against new version. Sorry it is release day and I am too busy to verify this myself but try 1.7.20 and let us know.

@Xanthus
Copy link
Author

Xanthus commented Jun 10, 2015

I imagined something like that would be the case. 1.7.20 also fails it seems.

@enebo
Copy link
Member

enebo commented Jun 10, 2015

oh great. Thanks for checking. I think if you make a more elaborate require hook you can work around this but we will definitely try and fix this for 1.7.21.

@enebo enebo added this to the JRuby 1.7.21 milestone Jun 10, 2015
@Xanthus
Copy link
Author

Xanthus commented Jun 10, 2015

Yip, it was easy enough to work around once I realised what the problem was. Thanks for the quick response!

@enebo enebo modified the milestones: JRuby 1.7.21, JRuby 1.7.22 Jul 7, 2015
@enebo enebo modified the milestones: JRuby 1.7.22, JRuby 1.7.23 Aug 20, 2015
@enebo enebo modified the milestones: JRuby 1.7.23, JRuby 1.7.24 Nov 24, 2015
@headius
Copy link
Member

headius commented Jan 15, 2016

So the problem is that the MiddleClass file runs twice, wiping out the @registry hash, right?

I'll poke at this a bit. We've had plenty of challenges with double-requiring files over the years.

@headius
Copy link
Member

headius commented Jan 15, 2016

This appears to have been fixed some time between 1.7.21 and now. Here's the jruby-1_7 branch:

[] ~/projects/jruby-1.7 $ jruby blah.rb
rm: base_class.rb: No such file or directory
Files created, run "ruby base_class.rb to test"

[] ~/projects/jruby-1.7 $ jruby base_class.rb 
BaseClass loaded
MiddleClass loaded
Subclass1 loaded
Subclass2 loaded
All good here

And here's master:

[] ~/projects/jruby $ jruby blah.rb
rm: base_class.rb: No such file or directory
rm: subclasses: No such file or directory
Files created, run "ruby base_class.rb to test"

[] ~/projects/jruby $ jruby base_class.rb 
BaseClass loaded
MiddleClass loaded
Subclass1 loaded
Subclass2 loaded
All good here

I did not test JRuby 1.7 in Ruby 1.8 mode because 1.8.7 does not make guarantees about loading the same file with different relative paths.

Calling this one fixed as of 1.7.24. Please verify @Xanthus, thank you!

@headius headius closed this as completed Jan 15, 2016
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

3 participants