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

Autowiring throws NullPointerException [SPR-13599] #18177

Closed
spring-issuemaster opened this Issue Oct 22, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Oct 22, 2015

Michael Schoene opened SPR-13599 and commented

After switching to 4.2.2 I get the following exception during autowiring:

Caused by: org.springframework.beans.factory.BeanCreationException: Could not autowire field: private java.lang.String de.ei.janet.server.Janet.saDir; nested exception is java.lang.NullPointerException
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:571) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	at org.springframework.beans.factory.annotation.InjectionMetadata.inject(InjectionMetadata.java:88) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor.postProcessPropertyValues(AutowiredAnnotationBeanPostProcessor.java:331) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	... 29 common frames omitted
Caused by: java.lang.NullPointerException: null
	at java.util.concurrent.ConcurrentHashMap.containsValue(ConcurrentHashMap.java:979) ~[na:1.8.0_60]
	at java.util.concurrent.ConcurrentHashMap$ValuesView.contains(ConcurrentHashMap.java:4664) ~[na:1.8.0_60]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.determineAutowireCandidate(DefaultListableBeanFactory.java:1228) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1124) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1014) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	at org.springframework.beans.factory.annotation.AutowiredAnnotationBeanPostProcessor$AutowiredFieldElement.inject(AutowiredAnnotationBeanPostProcessor.java:543) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
	... 31 common frames omitted

The reason seams to be the last changes in DefaultListableBeanFactory (Commit 097bcfb). This commit replaces HashMap with ConcurrentHashMap. After this change the fallback case in method determineAutowireCandidate throws the NullPointerException if beanInstance is null.


Affects: 4.2.2

Referenced from: commits 5386e8a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 23, 2015

Stéphane Nicoll commented

Could you please share a sample project that exhibits the problem?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 23, 2015

Michael Schoene commented

Upload samle project to github: https://github.com/mrs2207/spr-13599.git

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 23, 2015

Stéphane Nicoll commented

Thanks for the sample project. A few remarks:

  • the PropertySourcesPlaceholderConfigurer is actually useless.
  • You are autowiring a collaborator in the Spring Boot application itself so we are forced to eagerly initialize it. If I move those two strings to a separate service, the application runs fine. We are recommending not to inject dependencies in the Spring Boot application class itself.

Since it is a Spring Boot sample, I'd be tempted to move that issue in the Spring Boot tracker, but I'd like Juergen Hoeller to have a look first.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 23, 2015

Michael Schoene commented

I updated my sample project (move the autowired strings to a separate service). The exception is still being thrown.

Why is the PropertySourcesPlaceholderConfigurer useless? Without this bean and the line

pspc.setNullValue( "@null" );

the @null default value in the Properties class does not work as expected.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 23, 2015

Juergen Hoeller commented

On review, there seems to be an issue with our use of containsValue here: Even if our Map never holds a null value, we must not call containsValue with a null - which would return false in any case but is not supported that way by ConcurrentHashMap. And since bean instances may be null in certain rare cases, we need to be defensive there. To be addressed for 4.2.3 (due in about two weeks).

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 26, 2015

Juergen Hoeller commented

Turns out that my assumption was right: The presence of a null-returning FactoryBean whose getObjectType() result matches a given injection point, with more than one matching bean present, can lead to a NullPointerException if there is no primary match otherwise... e.g. if the FactoryBean result would be chosen based on a bean name match. I've added a unit test for this and made the determineAutowireCandidate more defensive now, simply through only calling ConcurrentHashMap.contains in case of a non-null bean instance.

A general lesson here: It's not sufficient to check whether a given Map may actually contain null values before turning it into a ConcurrentHashMap (or other impl which doesn't allow null values). We also need to make sure that nobody calls containsValue / values().contains with a null argument since such impls will throw a hard NPE there.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment