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
Don't allow PlatformClassLoader
or AppClassLoader
as override classloaders
#795
Comments
Hi, sorry this took me forever to get to! I think I fixed the problem, but please take a look at my changes and let me know if I understood properly what you were explaining... |
Hey, thank you for looking into it! I realize my original issue was not worded the best, I appreciate you taking the time to sort through it. I am also trying to refresh my memory and understand the changes you made. One thing I noticed which confused me is this: Here it seems like |
So assuming setting
This change seems to make sense to me. The AppClassLoader is for the regular user-defined classpath, not for JDK platform classes.
This also makes sense. If the user specifes the classloader to be scanned to be the AppClassLoader but also specifices to ignore parent classloaders, that would mean that the user expects the Platform classloader and the Boostrap classloader to be ignored. The classpath is loaded by the AppClassLoader, which would not be ignored in this case since only parents of it are ignored, so it makes sense for it the classpath to be scanned in this case. Though I am curious about one thing which could be another issue. If only one classloader is specified, and it is the |
Oops, good catch! Thanks, fixed. |
Correct on your points 1 and 2. I hope this doesn't break things for other users, but I think the changes make sense. To be honest though I left the Java world about 18 months ago, and I'm getting rusty on this stuff now, so I'll rely on your judgment as to whether these are appropriate changes!
Because in JDK 9+, it is impossible to get non-module (traditional) classpath entries from the classloaders. The classes are encapsulated against reflection, and they don't expose the URLs of the traditional classpath, because they don't extend Please check if this now satisfies your expectations, after the fix and the above explanation. Thanks! |
So to put it in other words, Classgraph is just taking the PlatformClassLoader and AppClassLoader as tokens, and if it receives these tokens it gets all of the info from the module API? And the module API does not distinguish between platform and classpath classes, or at least you are not distinguishing platform and classpath classes in your usage of the module API? And the If this is true, then I feel caught up and I think that this can be closed! If we were to design a Classgraph 5.0 API, I might suggest a (breaking) change that would just clarify all of this directly in the API. I think I would throw an exception if the But for now, I am content. Thank you, and congratulations for escaping Java :) |
Hah, thanks -- I found a new home in Dart -- and Dart is so much better than Java that I never want to touch Java again if I can avoid it! At some point I'm going to try to find a new home for ClassGraph, because I won't be able to maintain it forever.
Well most of the time, nobody is trying to override the classloader with a system classloader (either of these two). They have a custom classloader that they want to scan. That's what the API is for: for throwing away all the environment classloaders, and scanning another classloader that is not part of the runtime environment (e.g. you could instantiate your own If one of the two JDK9+ classloaders is detected in the runtime environment, and if the environment classloaders are not overridden (as you are trying to do), then ClassGraph will enable module scanning, through the module API, unless it has been manually disabled. But yes, basically there is no way to scan anything at all in Actually because of strong encapsulation in JDK 9+ (which started to be enforced in JDK 16), I wrote Narcissus: https://github.com/toolfactory/narcissus This uses the JNI API to override all Java security. You can actually use this during scanning, if you have any classloader that does not expose the classpath publicly, but has a classpath in some private field, and you're running on JDK 16+: https://github.com/classgraph/classgraph?tab=readme-ov-file#running-on-jdk-16 This could probably be used to read the classpath from As far as the module scanning API, it is possible that one or both of these system classloaders can contain module layers that are not visible to the running program, and maybe you would want to scan these too, but it would be far too complex (and not future-proof) to try to get around module visibility limitations using Narcissus-based reflection. So I didn't bother doing this either, and the module scanner can only scan modules that are in module layers that are visible to the running code. Anyway, these are the complex reasons for the logic surrounding these classloaders...!
Half the open bugs are TODO items for Classgraph 5.0, so I'll keep this bug open for now, but I'll change the subject line to reflect this.
Great, thanks for verifying. I pushed out a bugfix release for the bug you noticed, as 4.8.169. Thanks for your eagle-eyed checking over the code changes! |
ignoreParentClassLoaders()
PlatformClassLoader
or AppClassLoader
as override classloaders
So, note to self, this is what this issue is now about:
However, I might take this further, and actually require the user to specify exactly which classloaders they want to scan, and which classpath specification mechanisms (classpath or module API), otherwise ClassGraph should not scan anything by default. That way there is no confusion over what will be scanned. |
Interesting! I know nothing about Dart. I purely use Kotlin now, so I've escaped Java too but I still find the JVM and java libraries and frameworks useful.
I have to admit I don't 100% understand the complexities of this, but I really appreciate that you took the time to write it. If I ever have any confusion about this behavior of Classgraph I can come back here and review this. Narcissus looks cool. There have been one or two times I had to access an inernal class (like
This sounds great! I appreciate that you're considering this. I think users would benefit a lot from these changes because it would greatly clarify the scanning mechanisms being used. I know I would prefer a more clear API (even if it increased the number of lines of code it took do the same thing, I would prefer clarity.) |
Well Narcissus is an abominable hack, it's only there if you really need it, and Oracle could close this loophole at any time. But frankly at this point I have enough scars from Java that I don't really care what happens in the Java world in the future! 😁 |
Hello, first of all thanks for the great library. Its really a pleasure using it.
I'm having an issue and have been searching this repo for answers. I found #639 and that seems to be closely related to my current issue.
I am overriding the classloaders with
jdk.internal.loader.ClassLoaders$AppClassLoader
. I expcted to get all of my classes, but the the list of classes returned is empty. At this point, I felt confused and knew something was wrong.This seemed wrong because clearly many classes were loaded with the system classloader. I checked and made sure, and in fact all of the classes I was looking for were loaded from it.
Eventually, I disabled
ignoreParentClassLoaders
, and then it worked as expected.This seems to be a consequence of this line:
classgraph/src/main/java/nonapi/io/github/classgraph/classpath/ClasspathFinder.java
Line 298 in 5f94ed6
It seems like there have been issues with the system classloader, and that these issues have been addressed but only for when
ignoreParentClassLoaders
is false. However, I think many new users such as myself might intuitively useignoreParentClassLoaders
with the System classloader in an attempt to avoid using the platform classloader.So there is a perfectly fine workaround, but I think something might be inconsistent here in the library. I think if we configure ClassGraph to use only the system classoader but to "ignore parent classloaders", I think we would expect it to get just that- all the classed loaded by the System classloader without the platform classes loaded with the platform loader.
After finding the option
enableSystemJarsAndModules()
, I realized that platform classes are not loaded by default. That makes it so I no longer need to useignoreParentClassLoaders
. So my workaround of just not usingignoreParentClassLoaders
is perfectly fine. but I just think maybe there should be a warning or error for users who try to do what I did otherwise our expectations would not be met leading to confusion.Using classgraph version 4.8.162.
One thing I think that really clarifies this issue to me is that in the log, this statement is inconsistent:
So the log says that it notices we used the System classloader and did an automatic fix for it. (I think the log has a typo, it says
java.lang.path
but I think it meantjava.class.path
?) But the according to the source code, I should expect to see the lineGetting classpath entries from java.class.path
in my logs, but I never do.The text was updated successfully, but these errors were encountered: