Update classgraph dependency to start up in low-memory environments

[TWiStErRob]

See classgraph/classgraph#400 for more details.

Description

Bump a version number of classgraph so that Neo4J OGM can work in lower-memory setups easily. It should be a minor change as it was updated only 4 releases ago which was a month ago.

Related Issue

There's no issue in this repo, the "Background" in classgraph/classgraph#400 is the issue.

Motivation and Context

I'm following up on classgraph/classgraph#400 (comment)
4.8.63 has a fix for an infinite loop on OOM and significantly lowers the memory consumption of classgraph itself. But it doesn't work on JDK7-8, so using the latest 4.8.64 which fixes that compatibility issue.

How Has This Been Tested?

I have a project where I overrode the classloader dependency with

allprojects {
	configurations.all {
		resolutionStrategy.dependencySubstitution.all {
			val requested = this.requested
			if (requested is ModuleComponentSelector
				&& requested.module == "classgraph"
				&& requested.group == "io.github.classgraph"
			) {
				if (VersionNumber.parse(requested.version) < VersionNumber.parse("4.8.62")) {
					// 4.8.63 doesn't work on JDK 8
					useTarget(
						"${requested.group}:${requested.module}:4.8.64",
						"Performance issue with low memory: https://github.com/classgraph/classgraph/issues/400"
					)
				}
			}
		}
	}
}

My harness integration tests pass and app works as before on JDK 8 and -Xmx64M (previously it was sometimes failing with -Xmx256M and consistently failing with -Xmx128M).

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
  (I still didn't read it as it still doesn't exist... I commented on this in Clean up built-in conversions documentation #501 too :)
• I have added tests to cover my changes.
• All new and existing tests passed.
  I'll leave this up to the CI

[TWiStErRob]

@michael-simons I also noticed that the last change to this was #735 and this comment (from an earlier time):

neo4j-ogm/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java

Lines 84 to 86 in 5baa143

	// We cannot use the ClassGraph class loader as the order of class loaders used is broken since
	// ClassGraph 4.8.19 again when `org.springframework.boot.devtools.restart.classloader.RestartClassLoader`
	// is present.

Sounds like your work could be related to a fix in 4.8.62:

Classloader delegation order now respects classpath and classloader override settings configured before starting the scan.

and also maybe is an outdated comment since the code after it does use classgraph.

[lukehutch]

Sounds like your work could be related to a fix in 4.8.62:

Yes, that comment is outdated, as the issue with classloading was fixed after this report by @michael-simons:

classgraph/classgraph#396

Please ensure you test ClassGraph 4.8.64 with Neo4J-OGM thoroughly before pushing out a release to your users, as this release contains some of the deepest changes that ClassGraph has undergone in a long time. All the unit tests pass fine on all platforms with this latest verison; however, I want to ensure there is no disruption to Neo4J-OGM users. (Apologies for the couple of recent disruptions that Neo4J-OGM has run into as a result of ClassGraph changes!)

[michael-simons]

Hey everyone. First of all, I absolutely love the thoughts, work and effort put in this ticket and the issues on Classgraph. I'm gonna grab that branch here and build it on our CI. Will take me a bit…

[michael-simons]

Re

and also maybe is an outdated comment since the code after it does use classgraph.

Yes, we use Classgraph, but only the scanning features at the moment and use our own class loader configuration.

neo4j-ogm/core/src/main/java/org/neo4j/ogm/metadata/DomainInfo.java

Lines 87 to 91 in 5baa143

	ClassLoader classLoader = Configuration.getDefaultClassLoader();
	try (ScanResult scanResult = findClasses(packages)) {
	for (String className : scanResult.getAllClasses().getNames()) {
	try {
	Class<?> clazz = Class.forName(className, false, classLoader);

While I noticed that Luke fixed the issues we had (thanks for that), we decided it would be better to take the class loading issues into our responsibility, so that we can make configurable in the future or at least provide a strategy.

I'm gonna adapt the comment, though.

[lukehutch]

While I noticed that Luke fixed the issues we had (thanks for that), we decided it would be better to take the class loading issues into our responsibility, so that we can make configurable in the future or at least provide a strategy.

That would be a reasonable thing to do, except that you have to make sure that the classes found by ClassGraph and the classes loaded by your own classloader are not just the same classes (e.g. what if ClassGraph can find some classes that your classloader can't, or vice versa?), but the same definitions of the same classes (i.e. that classpath elements are processed in the same order, so you don't get ClassGraph scanning one version of a class and your classloader loading a different version of the class). This has been a source of hard-to-trace bugs for some users of ClassGraph in the past.

One way you can reconcile what ClassGraph scans with what your context classloader loads is simply to override the classloaders that ClassGraph scans (this also overrides the classpath):

new ClassGraph().overrideClassLoaders(Configuration.getDefaultClassLoader())...

If you do decide to use ClassGraphClassLoader, I think things should work now and should continue to work in the future. The latest ClassGraphClassLoader is about as sane as I think it can be made. It defers to the context classloader(s) wherever possible. So if Configuration.getDefaultClassLoader() returns the context classloader (or a descendant of the context classloader), it should work.

https://github.com/classgraph/classgraph/blob/master/src/main/java/io/github/classgraph/ClassGraphClassLoader.java#L131

The only case where ClassGraphClassLoader should fall through to using Resource objects to load classfile bytecodes and define classes is when you are scanning something like a Spring-Boot jar, but outside the Spring-Boot environment (so that the context classloader doesn't even know how to scan classes in the Spring-Boot classpath or its nested lib jars).

[michael-simons]

Thanks, Luke.

I tried that right now

 public static DomainInfo create(TypeSystem typeSystem, String... packages) {

        DomainInfo domainInfo = new DomainInfo(typeSystem);
        Predicate<Class<?>> classIsMappable = clazz -> !(clazz.isAnnotation() || clazz.isAnonymousClass() || clazz
            .equals(Object.class));

        // We cannot use the ClassGraph class loader as the order of class loaders used is broken since
        // ClassGraph 4.8.19 again when `org.springframework.boot.devtools.restart.classloader.RestartClassLoader`
        // is present.
        ClassLoader classLoader = Configuration.getDefaultClassLoader();
        try (ScanResult scanResult = findClasses(packages)) {
            for (Class<?> clazz : scanResult.getAllClasses().loadClasses()) {
                if (!classIsMappable.test(clazz)) {
                    continue;
                }
                domainInfo.addClass(clazz);
            }
        } finally {
            domainInfo.finish();
        }
        return domainInfo;
    }

    private static ScanResult findClasses(String[] packagesOrClasses) {

        // .enableExternalClasses() is not needed, as the super classes are loaded anywhere when the class is loaded.
        return new ClassGraph()
            .overrideClassLoaders(Configuration.getDefaultClassLoader())
            .ignoreClassVisibility()
            .whitelistPackages(packagesOrClasses)
            .whitelistClasses(packagesOrClasses)
            .scan();
    }

But that still breaks further down the line in a Spring Boot reload scenario, as the underlying class loader has been swapped out for a new one.

Issue I see in such an application when accessing an entity for example is

java.lang.IllegalArgumentException: Can not set java.lang.Long field com.example.gh729.Thing.id to com.example.gh729.Thing
	at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167) ~[na:1.8.0_192]
	at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171) ~[na:1.8.0_192]
	at sun.reflect.UnsafeFieldAccessorImpl.ensureObj(UnsafeFieldAccessorImpl.java:58) ~[na:1.8.0_192]
	at sun.reflect.UnsafeObjectFieldAccessorImpl.get(UnsafeObjectFieldAccessorImpl.java:36) ~[na:1.8.0_192]
	at java.lang.reflect.Field.get(Field.java:393) ~[na:1.8.0_192]

I'm rather update class graph (looks good) and stick with our loading for now.

[lukehutch]

Fair enough, thanks for trying it. Your situation is unique enough that you should do what you need to do, as long as you understand the caveat that ClassGraph may find different classes from the ones that are loaded.

[michael-simons]

Just pushed the improved comment moments before your comment :)

[lukehutch]

Just edited my comment moments before you replied :-D

[lukehutch]

as long as you understand the caveat that ClassGraph may find different classes from the ones that are loaded.

Just to clarify -- primarily there are three situations:

1. ClassGraph may not find a class that Configuration.getDefaultClassLoader() is able to load. This might be able to be ameliorated in some cases by at least calling new ClassGraph().addClassLoader(Configuration.getDefaultClassLoader()), so that if Configuration.getDefaultClassLoader() is not found as one of the context classloaders, ClassGraph can be informed of its existence, so it can try to extract the classpath entries from that classloader.
2. ClassGraph may find a class that Configuration.getDefaultClassLoader() is not able to load. This could give you a ClassNotFoundException when you call Configuration.getDefaultClassLoader().loadClass(classInfo.getName()). If that call fails, maybe you could fall back to calling ClassGraph's classInfo.loadClass(), for maximum robustness. (Yes, this will still potentially fail in the case that the Spring Boot Restart classloader has been switched out.) The gotcha here though is that a class' entire class hierarchy has to be loaded in the same classloader: if a superclass and one of its subclasses are loaded in different classloaders, then the subclass cannot be cast to the superclass.
3. ClassGraph may find a different definition of a class than the one that Configuration.getDefaultClassLoader() loads. This will be the case when the class is defined multiple times on the classpath, and the classpath order found by ClassGraph is different from the classpath resolution order of Configuration.getDefaultClassLoader(). This may mean that you'll find a class based on some scanning criteria, but then the loaded class doesn't meet those criteria. Just keep an eye out for this, there's not much you can do about it if you do start using different classpaths for scanning and classloading.