ConfigurationClassParser needs to load annotations through source class loader [SPR-10343]

[spring-projects-issues]

Olaf Otto opened SPR-10343 and commented

A change from 3.1.3.RELEASE to 3.1.4.RELEASE in org.springframework.context.annotation.ConfigurationClassParser causes the getImport method to recursively load all annotations of a class(looking for @Import), using a metadataReaderFactory solely relying on the classloader of the annotated type. Resulting, the method fails in environments with classloader isolation (i.e. OSGi):

// the package containing @MyAnnotation is explicitly
// imported in the bundle containing AnnotatedType
@MyAnnotation 
public class AnnotatedType {
}
// This meta annotation stems from the spring bundle, i.e. is a transitive 
// import from @MyAnnotation and not explicitly imported into the bundle containing "AnnotatedType"
@Scope("prototype") 
public @interface MyAnnotation {
}

Resulting exception as the SimpleMetadataReader tries to obtain @Scope from the classloader containing AnnotatedType:

-- snipp --
Caused by: java.io.FileNotFoundException: class path resource [org/springframework/context/annotation/Scope.class] cannot be opened because it does not exist
	at org.springframework.core.io.ClassPathResource.getInputStream(ClassPathResource.java:157)
	at org.springframework.core.type.classreading.SimpleMetadataReader.<init>(SimpleMetadataReader.java:49)
	at org.springframework.core.type.classreading.SimpleMetadataReaderFactory.getMetadataReader(SimpleMetadataReaderFactory.java:80)
	at org.springframework.core.type.classreading.CachingMetadataReaderFactory.getMetadataReader(CachingMetadataReaderFactory.java:101)
	at org.springframework.core.type.classreading.SimpleMetadataReaderFactory.getMetadataReader(SimpleMetadataReaderFactory.java:76)
	at org.springframework.context.annotation.ConfigurationClassParser.getImports(ConfigurationClassParser.java:291)
	at org.springframework.context.annotation.ConfigurationClassParser.getImports(ConfigurationClassParser.java:293)
	at org.springframework.context.annotation.ConfigurationClassParser.getImports(ConfigurationClassParser.java:293)
	at org.springframework.context.annotation.ConfigurationClassParser.doProcessConfigurationClass(ConfigurationClassParser.java:223)
	at org.springframework.context.annotation.ConfigurationClassParser.processConfigurationClass(ConfigurationClassParser.java:147)
	at org.springframework.context.annotation.ConfigurationClassParser.parse(ConfigurationClassParser.java:124)
	at org.springframework.context.annotation.ConfigurationClassPostProcessor.processConfigBeanDefinitions(ConfigurationClassPostProcessor.java:289)
	... 12 more

The 3.1.3 implementation relied on the annotation metadata of the annotation directly applied to the processed type - perhaps it is possible to reintroduce such an approach in favor of the recursion in getImports.

---

Affects: 3.1.4, 4.2.5, 4.3.1

Attachments:

• spr-10343-consuming.zip (3.13 kB)
• spr-10343-declaring.zip (2.37 kB)

Issue Links:

• Prevent duplicate @Import processing and ImportBeanDefinitionRegistrar invocation [SPR-9925] #14558 Prevent duplicate @Import processing and ImportBeanDefinitionRegistrar invocation
• Do not fail with a circular @Import error caused by an @ComponentScan [SPR-14517] #19086 Do not fail with a circular @Import error caused by an @ComponentScan
• ConfigurationClassParser does not work with scanner-provided ClassLoader [SPR-15245] #19810 ConfigurationClassParser does not work with scanner-provided ClassLoader

Backported to: 4.2.8

0 votes, 6 watchers

[spring-projects-issues]

Chris Beams commented

Hi Olaf,

Isn't the most correct solution to this problem to add an import statement for org.springframework.context.annotation to the OSGi manifest of the bundle that declares MyAnnotation? It is something of an accident that this error wasn't raised before; i.e. the fix introduced in #14558 is a more correct way of processing annotations and meta-annotations, and as a side effect that it forces OSGi-based applications into more strictly correct manifest declarations.

Just to ensure that we're thinking about this the same way, if you ran BND or a similar tool over the bundle in question, wouldn't it detect the @MyAnnotation->@Scope dependency and create an Import: statement in the manifest by default?

I'll leave this issue open for the moment, in case I'm missing something. Please let us know whether you agree with the assessment above, thanks.

[spring-projects-issues]

Olaf Otto commented

Hello Chris

What do you mean by "declaring"? Should the bundle consuming the annotation also explicitely import the transitive dependencies of the annotation? The bundle containing @MyAnnotation does import org.springframework.context.annotation - am I mistaken by assuming that spring should operate on the classloader of the type it reflects on instead on the bundle classloader?

We are using the maven-bundle-plugin (BND) which does not explicitely add such transitive dependencies. However, it is my understanding that this is not necessary since the dependencies of an imported type are obtained from the imported type's classloader. Does this not apply to annotations?

Regards,
Olaf

[spring-projects-issues]

Olaf Otto commented

Added two example projects for reproduction. declaring: Provides @MyAnnotation, consuming: Uses @MyAnnotation. Both use maven-bundle-plugin (BND) to generate the respective OSGi exports/imports. The consuming bundle is equipped with a blueprint XML - instantiating this with Spring 3.1.4.release should provoke the issue.

[spring-projects-issues]

Chris Beams commented

Hi Olaf, I've asked a colleague more knowledgable than myself on OSGi matters to take a look at this issue and comment. We'll get back to you.

[spring-projects-issues]

Glyn Normington commented

bnd (and bundlor) automatically generates imports only for the packages of types which are referenced directly by the bundle in question. Transitive imports should not be necessary in general because Java's normal rule is to initiate the class load for a class X using the defining class loader of the class which refers to X.

In the above example, the class loader of the bundle containing AnnotatedType should be used to initiate the class load of MyAnnotation and that bundle merely needs to import the package of MyAnnotation. Similarly, the class loader of the bundle containing MyAnnotation should be used to initiate the class load of Scope and that bundle needs to import the package of Scope, which it does.

So I would say the 3.1.4 behaviour is a bug. I think the Spring framework code needs to initiate class loads relative to the appropriate defining class loaders.

[spring-projects-issues]

Olaf Otto commented

Gly, thank you for the clarification! I would of course appreciate to see the fix in a 3.1.5 release as it keeps me from upgrading from 3.1.3 ;-)

[spring-projects-issues]

Olaf Otto commented

Hello Chris

I have re-tested this with Spring 3.2.4.RELEASE and the issue is no longer reproducible. It appears the refactoring applied in this area on the 3.2 branch resolved this issue. In other words, please mark as resolved.

Thanks!
Olaf

[spring-projects-issues]

Olaf Otto commented

I have hit the same issue again using Spring 4.2.5 - re-opening. The issue arises due to the behavior of the ConfigurationClassParser, which does not use the source class classloader to load annotations, but rather the default resource loader:

private SourceClass getRelated(String className) {
...

if (this.source instanceof Class<?>) {
	try {
		Class<?> clazz = resourceLoader.getClassLoader().loadClass(className);
		return asSourceClass(clazz);
	}
...

IMO, this could rather be

private SourceClass getRelated(String className) {
...

if (this.source instanceof Class<?>) {
	try {
		Class<?> clazz = ((Class<?>) this.source).getClassLoader().loadClass(className);
		return asSourceClass(clazz);
	}
...

[spring-projects-issues]

Juergen Hoeller commented

Good point, the source class loader is definitely the most appropriate choice there. Fixed for 4.3.2 now, to be backported to 4.2.8.

[spring-projects-issues]

Adrian J George III commented

I've got a very similar issue in 4.3.6.
This line

spring-framework/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

Line 627 in 5b98a54

return asSourceClass(metadata.getClassName());

calls through to

spring-framework/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

Line 659 in 5b98a54

public SourceClass asSourceClass(String className) throws IOException {

and completely abandons the classloader the class was initially read from.
Unsurprisingly, this leads to the classloader that is being used to completely fail to find the class.
the problem can be reproduced here:
https://github.com/adrianjgeorge/classloading-poc

[spring-projects-issues]

Juergen Hoeller commented

Adrian J George III, please create a separate JIRA ticket for this. I'll have a look at it for 4.3.7.

[spring-projects-issues]

Adrian J George III commented

Will do.

[spring-projects-issues]

Adrian J George III commented

Sorry for so many comments, I'm surprised i can't edit. Issue is here #19810