jpmml/jpmml-evaluator

Problem with loadServiceProviderClasses using getContextClassLoader

Closed this issue · 3 comments

I've rarely had any trouble with your evaluator but once I upgraded to the latest version, I encountered a major problem which I have finally tracked down to the usage of classloaders to load resources.

My scenario is a bit odd, but we use Hitachi's (Pentaho) Kettle tool to run some PMML evaluations. This bit of code caused me some problems (in ServiceFactory.java):

ClassLoader clazzLoader = thread.getContextClassLoader();
if(clazzLoader == null){

This is in loadServiceProviderClasses which is a static method.

The problem is that the contextClassLoader in this scenario isn't the correct classloader to be used. Can I suggest something akin to this be used instead?

ClassLoader clazzLoader = serviceClazz.getClassLoader();
if (clazzLoader == null) {
  clazzLoader = thread.getContextClassLoader();
}
if (clazzLoader == null) {
 clazzLoader = ClassLoader.getSystemClassLoader();
}

So that the service class which is being passed in is in charge of loading the classes involved?

I've tested is in my various scenarios and it works fine (plus all of your existing tests run clean as well). I'm happy to do a pull request if this would be helpful.

The idea is to use the ClassLoader instance that loaded the current Java Application (parent/container).

The JPMML-Evaluator library is moving in direction that the pmml-evaluator artifact will be defining core classes, and individual model type implementations will be extracted into standalone JAR sub-libraries. The "runtime composition" for each Java application will be different (what model types/enhancements have been activated).

If the clazzLoader variable is tied to this ClassLoader instance that loaded the org.jpmml.evaluator.ModelEvaluator class definition, then the above plug-in mechanism might break.

Sure, using Thread#getContextClassLoader() goes against best practices. The assumption is that this class loader knows about the main Java application class (and its classpath). In your case, this class loader is probably some Web/Application Container class loader, which knows about many off-topic classes, but not the most on-topic ones.

What's your complaint here? The class loader that is obtained via Thread#getContentClassLoader() does not exist, is restricted in some way, or something else?

The solution would be to make the clazzLoader variable externally "assignable", probably by overloading key utility methods with an additional ClassLoader parameter.

On the end-user API level there would be some ModelEvaluatorBuilder setter method to set the initial value?

The problem I am encountering specifically is that the thread's "getContextClassLoader" isn't the correct class loader and doesn't have any of the correct classes, etc. This is because of the nature of Kettle's plugin architecture; the classloader it has is actually the classloader for the containing application (as opposed to the plugin which contains the evaluator code). I think being able to set the class loader from the api would be an excellent way of solving the problem as I could easily pass in my requested classloader.