[External] : Re: Local execution - class loader & class path issues

[Jan Lahoda]

On 15. 08. 23 16:41, Archie Cobbs wrote:

> Hi Jan,
>
> Thanks for taking a look.
>
> On Tue, Aug 15, 2023 at 8:12 AM Jan Lahoda <jan.lahoda at oracle.com> wrote:
>
>
>>     *Question*: Shouldn't LoaderDelegate.addToClasspath() have been
>>     passed the --class-path flag at some point in the example above?
>>     JShell's LocalExecutionControl uses DefaultLoaderDelegate, which
>>     uses a class loader of type
>>     DefaultLoaderDelegate.RemoteClassLoader. This would have added
>>     classes to RemoteClassLoader's class path.
>
>     Possibly.
>
> Thanks for the mild endorsement :) If you don't object I may file a 
> bug for this.


Sure, please do!


>
>>     *Question*: Shouldn't DefaultLoaderDelegate$RemoteClassLoaderuse
>>     the current thread's context loader as its parent loader instead
>>     of defaulting to the system class loader?
>
>     If that was done from the beginning, maybe. Now, probably not, as
>     that would be an incompatible change. Should be possible to have a
>     custom ExecutionControl (an instance of or subtype of
>     LocalExecutionControl), with custom LoaderDelegate (where,
>     possibly, there could be a factory for DefaultLoaderDelegate
>     taking a custom ClassLoader). Does not seem this should be very
>     too difficult?
>
> Yes, it's not too difficult and I've already done it (link 
> <https://urldefense.com/v3/__https://github.com/archiecobbs/java-console-toolkit/blob/main/jshell/src/main/java/org/dellroad/jct/jshell/LocalContextExecutionControlProvider.java__;!!ACWV5N9M2RV99hQ!NLf38Yz7eZhnS-RW-sdl_Rq5zTa9_4oYqZrKGZEtJm0olgVuLNX3UGDo22Xtt2e0Fdi3pvudhaBLhh6m8YjX7jY$>) 
> but it's still an annoying hassle...
>
> But I think we can get the best of both worlds, by adding a new 
> parameter that LocalExecutionControlProvider would accept, e.g. 
> contextLoaderParent=true. This would obviate the need for writing 
> custom a ExecutionControl but also not change the current behavior. Do 
> you see any issue with this idea?


If a new constructor for LocalExecutionControlProvider taking (probably) 
the ClassLoader would work, that would be a very good solution, I think.


>
>     (I am a bit confused about the command line example - at least the
>     "agent" (i.e. the DirectExecutionControl/LocalExecutionControl)
>     must run inside your target VM.)
>
> I was probably unclear. My point is that, if we're just looking at the 
> available JShell flags and parameters, you have zero control over the 
> execution class path when using local execution. The only control 
> available at all is the JVM classpath setting, which is what that 
> example shows.
>
> This is just a restatement of problem #1, i.e., the --class-path flag 
> is effectively broken when using local execution, because it affects 
> only the compilation class path, and not the execution class path.
>
>
>>     *Question*: Should JShell provide a way to "best effort" extract
>>     a compilation class path from a given ClassLoader (and its
>>     parents)? It could work for any URLClassLoader instances, but
>>     being part of the JDK it could also take advantage of
>>     inter-module access to the system and other built-in class
>>     loaders, which are all of type
>>     jdk.internal.loader.BuiltinClassLoader, which has an internal
>>     URLClassPath field.
>
>     Sorry, I don't think so. There has been some thinking some time
>     ago on supporting compilation based on ClassLoaders, but it is
>     fairly tricky, and very easy.
>
> I assume you meant "not very easy" there...


Yes, sorry.


>
> I'm curious to learn more.  Do you recall the upshot of that thinking? 
> Does it "work"? If not, what are the limitations?


Generally, as far as I know, ClassLoaders can load a resource/class 
given a name, but cannot provide a list of things under a 
directory/package. This causes two problems, IIRC:

- given a file containing only:

import does.not.exist.*;

I believe the specification says there should be an error for this 
import - but for that, we would need to know if there's a class (or 
possibly source) file under 'does.not.exist' or not. (There are some 
similar constrains on exports and esp. opens, but those are not relevant 
to JShell.)

- when the code contains something like "java.util.List", javac won't 
try to load "java.util.List". It will rather list the content of the 
whole "java.util" package, and look if there's a "List.class" (or alike) 
inside. If I remember correctly from the time I was experimenting with 
this, it is probably not impossible to change, but not easy, and 
consequences are not completely clear.


Jan


>     Having heuristics inside JShell to read some data from some
>     ClassLoaders does not sounds like a very good idea to me, sorry.
>
> I have to agree. This would be gross/hacky and I don't really like it 
> either (even though this is the workaround I'm currently being forced 
> to use).
>
> For the sake of argument, suppose we found a reliable way to make a 
> ClassLoader look like a JavaFileManager. Then all that would be 
> missing is a way to configure it in the JavaShellToolBuilder.
>
> Is there any particular reason why JavaShellToolBuilder couldn't have 
> a fileManager() method like JShell.Builder does?
>
> Thanks,
> -Archie
>
> -- 
> Archie L. Cobbs
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/kulla-dev/attachments/20230815/34425b0b/attachment.htm>