Graal: [native-agent] agent missing a type when computing reflect-config.json files

[aclement]

The repro project is here with instructions in the README:

https://github.com/aclement/spring-graal-agent-1

This is with graal 19.3 (11).

The type missing is org.springframework.context.support.PropertySourcesPlaceholderConfigurer - not quite sure which reflection call is not being caught though. That type is the return type of a method:

public static PropertySourcesPlaceholderConfigurer propertySourcesPlaceholderConfigurer() {

in type PropertyPlaceholderAutoConfiguration that is accessed via reflection

Object result = factoryMethod.invoke(factoryBean, args);

made from CglibSubclassingInstantiationStrategy:154. I suspect something is done reflectively on the result sometime later.

[peter-hofer]

The problem seems to be that FilteringSpringBootCondition.resolve() calls ClassLoader.loadClass() directly, while the agent only intercepts Class.forName(). Having the agent intercept ClassLoader methods seems unfeasible because the JVM and JDK also calls these methods internally, so it would observe many more than the reflectively used classes, and the recursive nature of class loader hierarchies would make it really difficult to filter by caller.
Can you patch FilteringSpringBootCondition.resolve() to use Class.forName() with a ClassLoader argument like ClassUtils.forName() does?

[aclement]

@peter-hofer as you can see from the linked image, looking to do something in Spring :) However, there are other places affected that I need to dig through. Are you not able to instrument the client side of the call to classloader.loadClass() and recognize it isn't from JDK code? I'm not against changing Spring but more concerned that it might miss places in developer application code unless they know they can't/shouldn't load classes that way.

[peter-hofer]

Instrumenting class loading should not be a problem. I imagine implementing this would entail doing a stack walk and detecting whether some implementation of loadClass is called directly from application or framework code, perhaps also indirectly through some kind of service provider mechanism, but never from another implementation of ClassLoader. We'd have to see how reliable this can be.

[peter-hofer]

In the upcoming 20.0 release, the agent will support an experimental-class-loader-support flag. With it, the agent instruments loadClass in all class loaders, tries to determine whether the method was explicitly called from outside the JDK, and if so and the class load/lookup is successful, adds the passed class name to the configuration. Use it like -agentlib:native-image-agent=experimental-class-loader-support,config-output-dir=/path/to/config-output/. Please give it a try and report on your experiences so we can improve and enable it by default.

[aclement]

That sounds great, will definitely try it. Is that in master so I could kick the tires on it with my own dev build?

[peter-hofer]

Not yet, but it is already queued to merge.

[peter-hofer]

Merged in 40920e4bed15aba75e2f906f310c66e1e8011ece

[peter-hofer]

@aclement @sbrannen you seem to be using this now, could you please give us some feedback? Do you think this is ready to be enabled by default now or did you run into any difficulties?

[aclement]

Hey. From my point of view I've used it a couple of times. It certainly found more stuff, but it didn't find the thing I needed in at least one situation. But I haven't seen it make things worse (perhaps feels weird that it finds much more stuff but those entries don't affect the application success). Spring Boot refactored to something that would be caught without this change so it hasn't been as urgent. I'll let Sam comment on whether it helped down at the framework module level.

[sbrannen]

TBH, I haven't noticed a difference in the two Spring modules that I modified for unit testing within a native image. I added the experimental-class-loader-support option with the hope that it would make my life easier (while I was still getting the tests to pass), but the tests I have in place still run without the option.

So it's obviously not making things worse as @aclement pointed out, and I assume it finds more stuff, but I'm not currently running into use cases that exercise that functionality.

[peter-hofer]

Thanks @aclement and @sbrannen! If you run into an issue where this option breaks something or doesn't seem to work as intended, please let us know.