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

Create friendlier API for swapping current thread context classloader #3090

Closed
enebo opened this Issue Jun 29, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@enebo
Copy link
Member

commented Jun 29, 2015

 java.lang.Thread.currentThread.setContextClassLoader( JRuby.runtime.jruby_classloader )

This works but since we have had 2-3 issues opened and we need to give a fairly complex snippet of code (above snippet) I think we should try and add something simpler to use:

require 'jruby'

JRuby.use_jruby_classloader

where:

def self.use_jruby_classloader(thread=java.lang.Thread.currentThread)
end

I spent about 10 seconds coming up with this so it is mostly just an attempt at getting people to think how we can make this simpler. Also do we need to care about more than context classloaders?

@enebo enebo added this to the JRuby 9.0.0.0.rc2 milestone Jun 29, 2015

@mkristian

This comment has been minimized.

Copy link
Member

commented Jun 29, 2015

it is really only about the context classloader since some frameworks just "assume" that this is where the application is running. OSGi has the same problem: http://njbartlett.name/2010/08/30/osgi-readiness-loading-classes.html

I would be just plain

JRuby.setup_context_classloader

where:

def self.setup_context_classloader
   java.lang.Thread.currentThread.setContextClassLoader( JRuby.runtime.jruby_classloader )
end
@mkristian

This comment has been minimized.

Copy link
Member

commented Jun 29, 2015

actually in case JRuby.runtime.jruby_classloader.parent the classloader which has jruby itself, in case this classloader is NOT the context classloader it would be nice to at least warn on possible side effects or even not set the context classloader.

@enebo

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2015

The name setup_context_classloader says it will setup the classloader but does nothing to imply with what it will set up with or to what it will set it. So I don't think the name really says what is going on. I do not love my name either though so I am just pointing out what I think is a naming issue. The other thought I had was whether we should add '!' onto the end to imply side-effect or definite change in behavior from that call onward.

@kares

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

how about just keeping the name the same "as is" :

JRuby.set_context_class_loader

+1 for allowing to pass a custom class-loader :

def JRuby.set_context_class_loader(loader = JRuby.runtime.jruby_class_loader)
  java.lang.Thread.currentThread.setContextClassLoader loader
end

naming should be consistent its JRuby.runtime.jruby_class_loader NOT _classloader

@enebo

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2015

@kares looking back at the original snippet I guess I agree with you that set_context_class_loader could match context_class_loader (which of course is currently named context_classloader atm). So unless @mkristian or anyone else has input here we should add a second method jruby_class_loader and then add the set method as you have above.

I am not sure if we should put this into the FAQ. We have had multiple issues opened about this change.

@mkristian

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

+1 about the naming and the implementation of @kares

regarding the FAQ it would be good to encourage people to first find a way to tell their java libarary which classloader to use before using this workaround.

@kares

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

great, go for it ... someone :)

regarding FAQ: people running into these should understand a bit of class-loaders (although for sure in one case it seemed not the case) since they import libraries which assume to be able to access the class-es across the class-path. for me no issue seemed "fatal" (for now) and actually some might benefit from changing how their class-path is set, would maybe mention some of it :

loading jars in 9K changed in way that the (thread) context class-loader is no longer changed when using jruby from the command-line. this was done to provide a more consistent behaviour across various ways of using JRuby (e.g. embed). most Java libraries should work fine as they are self-contained and do not depend on the caller's class being accessible to them. for those that assume and/or need to (reflectively) instantiate classes from JRuby or elsewhere on the class-path we recommend either using the -J-cp option or executing JRuby.set_context_class_loader to match JRuby 1.7.x behavior

@enebo enebo closed this in c61f2bb Jul 1, 2015

@enebo

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2015

No test written....but it is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.