ServiceLoader.load(Class, ClassLoader) does not load services exposed in modules and loaded by parent CL

[Peter Levart]

Hi Robert,

On 05/24/2018 07:33 PM, Robert Scholte wrote:
> Hi Peter,
>
> you've been hit by MNG-6371[1]. We've tried to solve this in Maven 
> 3.5.1, but we faced other unexpected classloader issues with 
> maven-extensions and maven-plugins with extensions. So far we haven't 
> been able to fix it.
> This is actually THE reason why Maven 3.5.1 was never released, we 
> reverted the classloader related changes and successfully released 
> Maven 3.5.2.
>
> If you like a challenge or a bit more info: the main issue is when we 
> create a new Realm with null[2]. This will create a new ClassLoader 
> with parent null[3], meaning no bootstrap classloader.

I haven't studied code much, so perhaps you could explain why you need 
to create such ClassLoader (ClassRealm) that has a null parent (when 
calling .getParent() upon it), but then you simulate the parent in a 
non-compatibe way:

         ClassRealm classRealm = newRealm( baseRealmId );

         if ( parent != null )
         {
             classRealm.setParentClassLoader( parent );
         }

Such ClassRealm does eventually delegate to parent or any other realm 
via strategy. It's just that such delegation is not via official 
getParent() but via a non-standard getParentClassLoader().

Why do you need the official getParent() to be null? I see that you use 
super.loadClass() in ClassRealm.loadClass() and perhaps you don't want 
that super call to "see" anything else than current class path assigned 
to current ClassRealm (a sublclass of URLClassLoader) and bootstrap 
classes. Unfortunatelly ClassLoader.getParent() is final and you can't 
just override it and return the parentClassLoader field...

     private Class<?> unsynchronizedLoadClass( String name, boolean 
resolve )
         throws ClassNotFoundException
     {
         try
         {
             // first, try loading bootstrap classes
             return super.loadClass( name, resolve );
         }
         catch ( ClassNotFoundException e )
         {
             // next, try loading via imports, self and parent as 
controlled by strategy
             return strategy.loadClass( name );
         }
     }

Instead of using super.loadClass() which 1st delegates to "official" 
parent (which you would like it to be null), you could re-implement that 
ClassLoader logic by "pretending" that official parent is null although 
it would be non-null. Instead of calling super.localClass( name, resolve 
), you could call loadClassWithoutParent( name, resolve ) which would be 
something like:

     private static final ClassLoder bootstrapDelegator = new 
ClassLoader(null);

     private Class<?> loadClassWithoutParent(String name, boolean resolve)
         throws ClassNotFoundException
     {
         synchronized (getClassLoadingLock(name)) {
             // First, check if the class has already been loaded
             Class<?> c = findLoadedClass(name);
             if (c == null) {
                 try {
                     c = bootstrapDelegator.loadClass(name);
                 } catch (ClassNotFoundException e) {
                     // ClassNotFoundException thrown if bootstrap class 
not found
                 }

                 if (c == null) {
                     // If not found, then invoke findClass in order
                     // to find the class.
                     c = findClass(name);
                 }
             }
             if (resolve) {
                 resolveClass(c);
             }
             return c;
         }
     }


Would that work?


Regards, Peter

>
> I can quote Alan:
> Rhino used to be the JS engine in older releases and that may have 
> been in rt.jar and so loaded by the boot loader. When Nashorn replaced 
> it (in JDK 8) then it was configured to be defined to the extension 
> class loader so this is why the code snippet doesn't find it.
>
> In the Maven mailinglist are several threads trying to define how 
> Maven Classloading should work. So far only a few have mentioned this 
> issue.
>
> IIRC some have worked around it by initializing a new classloader.
>
> thanks,
> Robert
>
>
> [1] https://issues.apache.org/jira/browse/MNG-6371
> [2] 
> https://github.com/apache/maven/blob/4b95ad9fce6dfe7eec2be88f5837e96c7fbd7292/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java#L123
> [3] 
> https://docs.oracle.com/javase/9/docs/api/java/lang/ClassLoader.html#ClassLoader-java.lang.String-java.lang.ClassLoader-
>
> On Thu, 24 May 2018 12:29:33 +0200, Alan Bateman 
> <Alan.Bateman at oracle.com> wrote:
>
>> On 23/05/2018 21:28, Peter Levart wrote:
>>> :
>>>
>>> It's not an official plugin. And it seems that the Maven container 
>>> is to blame, not the plugin.
>> Robert Scholte is on this mailing list and may be able to comment on 
>> this.
>>
>>
>>> The nonstandard ClassLoader is supplied by the container. The plugin 
>>> just uses the most direct and default API possible to instantiate 
>>> JavaScript engine:
>>>
>>> jsEngine = new ScriptEngineManager().getEngineByName("JavaScript");
>>>
>>> It is the environment the plugin is executing in that doesn't play 
>>> well with how system service providers are located from JDK 9 on - 
>>> namely, the nonstandard ClassLoader that delegates to system class 
>>> loader, but does not express this also in the .getParent() result. I 
>>> don't know why Maven choose this, but closer inspection reveals that 
>>> its ClassLoader does have a "parent", but it keeps it in its own 
>>> field called "parentClassLoader" and doesn't return it from 
>>> .getParent(). There must be a reason for this, but I don't know that 
>>> it is.
>>>
>>> Do other parts of the JDK also use TCCL to bootstrap service lookup 
>>> by default? Isn't it unusual that ScriptEngineManager uses TCCL by 
>>> default?
>> I wasn't involved in JSR 223 but it may have envisaged scenarios 
>> where applications bundle scripting language implementations. This is 
>> not too unusual and you'll find several APIs do this to allow for 
>> cases where an application is launched in a container environment. 
>> Legacy applet and Java EE containers have historically created a 
>> class loader per "application" and this becomes the TCCL for the 
>> threads in that application.
>>
>> -Alan