From 4e54c25179e38c235bfba1dfca93ebe350e247c0 Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Fri, 22 Jan 2021 02:33:26 +0800 Subject: [PATCH 1/4] Fail early FactoryBean instantiation for LinkageError In 0288878 to resolve gh-22409, a little bug was introduced: if there is LinkageError in FactoryBean instantiation, no first exception. In JVM, if a class cannot be initialized, it acts like this: - at the first time, it will show the real reason and stack - then, only show "NoClassDefFoundError: Could not initialize class xxx" --- .../AbstractAutowireCapableBeanFactory.java | 8 +++ ...gressiveFactoryBeanInstantiationTests.java | 64 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 4defe323d461..e018ab6f76b3 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1022,6 +1022,14 @@ private FactoryBean getSingletonFactoryBeanForTypeCheck(String beanName, Root throw ex; } catch (BeanCreationException ex) { + // LinkageError is unresolvable + Throwable cause = ex.getCause(); + while (cause != null) { + if (cause instanceof LinkageError) { + throw ex; + } + cause = cause.getCause(); + } // Instantiation failure, maybe too early... if (logger.isDebugEnabled()) { logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex); diff --git a/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java b/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java index 9bec24609797..76455af7a9e5 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java @@ -16,12 +16,19 @@ package org.springframework.context.annotation; +import java.io.ByteArrayOutputStream; +import java.io.PrintWriter; + import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; +import static org.assertj.core.api.Assertions.assertThat; + /** * @author Andy Wilkinson */ @@ -49,6 +56,23 @@ public void beanMethodFactoryBean() { } } + @Test + public void beanMethodFactoryBeanWithError() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) { + context.register(BeanMethodConfigurationWithError.class, StringTypeConfiguration.class); + try { + context.refresh(); + } catch (BeanCreationException ex) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintWriter pw = new PrintWriter(baos); + ex.printStackTrace(pw); + pw.flush(); + String stackTrace = baos.toString(); + assertThat(stackTrace.contains(SimpleLinkageErrorClass.class.getSimpleName() + ".")).isTrue(); + } + } + } + @Configuration static class BeanMethodConfiguration { @@ -60,6 +84,33 @@ public SimpleFactoryBean simpleFactoryBean(ApplicationContext applicationContext } + @Configuration + static class BeanMethodConfigurationWithError { + + @Bean + public SimpleFactoryBean simpleFactoryBean(ApplicationContext applicationContext) { + SimpleFactoryBean simpleFactoryBean = new SimpleFactoryBean(applicationContext); + // cause a linkage error + new SimpleLinkageErrorClass(); + return simpleFactoryBean; + } + } + + + @Configuration + static class StringTypeConfiguration { + + @Autowired + private String foo; + + @Bean + public String foo() { + return "foo"; + } + + } + + static class SimpleFactoryBean implements FactoryBean { public SimpleFactoryBean(ApplicationContext applicationContext) { @@ -76,4 +127,17 @@ public Class getObjectType() { } } + + static class SimpleLinkageErrorClass { + + private static final int ERROR = checkError(); + + public SimpleLinkageErrorClass() { + } + + private static int checkError() { + throw new NoSuchMethodError(); + } + } + } From 7579fb81f872c4c9fc48e5556cca4d41cb226420 Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Sat, 23 Jan 2021 16:19:10 +0800 Subject: [PATCH 2/4] don't suppress LinkageError during bean creation If there is exception in class initializer, jvm acts like this: - for the first time, show the real reason and stack - then, mark the class in error state, throws only NoClassDefFoundError This commit don't suppress any LinkageError during bean creation. --- .../support/AbstractAutowireCapableBeanFactory.java | 8 -------- .../factory/support/DefaultSingletonBeanRegistry.java | 10 ++++++++++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index e018ab6f76b3..4defe323d461 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1022,14 +1022,6 @@ private FactoryBean getSingletonFactoryBeanForTypeCheck(String beanName, Root throw ex; } catch (BeanCreationException ex) { - // LinkageError is unresolvable - Throwable cause = ex.getCause(); - while (cause != null) { - if (cause instanceof LinkageError) { - throw ex; - } - cause = cause.getCause(); - } // Instantiation failure, maybe too early... if (logger.isDebugEnabled()) { logger.debug("Bean creation exception on singleton FactoryBean type check: " + ex); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java index 0a81aef4e9fe..791d4a9a7229 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultSingletonBeanRegistry.java @@ -274,6 +274,16 @@ public Object getSingleton(String beanName, ObjectFactory singletonFactory) { * @see BeanCreationException#getRelatedCauses() */ protected void onSuppressedException(Exception ex) { + // never suppress unresolvable LinkageError during bean creation + if (BeanCreationException.class.equals(ex.getClass())) { + Throwable cause = ex.getCause(); + while (cause != null) { + if (cause instanceof LinkageError) { + throw (BeanCreationException) ex; + } + cause = cause.getCause(); + } + } synchronized (this.singletonObjects) { if (this.suppressedExceptions != null && this.suppressedExceptions.size() < SUPPRESSED_EXCEPTIONS_LIMIT) { this.suppressedExceptions.add(ex); From c4186b0e1b593662a29d1e93a46d74a2c54aa50e Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Sat, 23 Jan 2021 16:21:48 +0800 Subject: [PATCH 3/4] don't suppress LinkageError when set property If there is exception in class initializer, jvm acts like this: - for the first time, show the real reason and stack - then, mark the class in error state, throws only NoClassDefFoundError This commit don't suppress any LinkageError when set property --- .../springframework/beans/AbstractPropertyAccessor.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spring-beans/src/main/java/org/springframework/beans/AbstractPropertyAccessor.java b/spring-beans/src/main/java/org/springframework/beans/AbstractPropertyAccessor.java index 1d6b5f48eab9..41bf5d4cffac 100644 --- a/spring-beans/src/main/java/org/springframework/beans/AbstractPropertyAccessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/AbstractPropertyAccessor.java @@ -116,6 +116,14 @@ public void setPropertyValues(PropertyValues pvs, boolean ignoreUnknown, boolean // Otherwise, just ignore it and continue... } catch (PropertyAccessException ex) { + // never suppress unresolvable LinkageError + Throwable cause = ex.getCause(); + while (cause != null) { + if (cause instanceof LinkageError) { + throw ex; + } + cause = cause.getCause(); + } if (propertyAccessExceptions == null) { propertyAccessExceptions = new ArrayList<>(); } From 5f6297b4b990bf211486655964da0a5d1e92716e Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Sat, 23 Jan 2021 16:22:09 +0800 Subject: [PATCH 4/4] testcase for exception in initializer --- ...gressiveFactoryBeanInstantiationTests.java | 64 -------- .../ExceptionInInitializerTests.java | 154 ++++++++++++++++++ .../ExceptionInInitializerTests.xml | 11 ++ 3 files changed, 165 insertions(+), 64 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.java create mode 100644 spring-context/src/test/resources/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.xml diff --git a/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java b/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java index 76455af7a9e5..9bec24609797 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/AggressiveFactoryBeanInstantiationTests.java @@ -16,19 +16,12 @@ package org.springframework.context.annotation; -import java.io.ByteArrayOutputStream; -import java.io.PrintWriter; - import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.FactoryBean; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; -import static org.assertj.core.api.Assertions.assertThat; - /** * @author Andy Wilkinson */ @@ -56,23 +49,6 @@ public void beanMethodFactoryBean() { } } - @Test - public void beanMethodFactoryBeanWithError() { - try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) { - context.register(BeanMethodConfigurationWithError.class, StringTypeConfiguration.class); - try { - context.refresh(); - } catch (BeanCreationException ex) { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - PrintWriter pw = new PrintWriter(baos); - ex.printStackTrace(pw); - pw.flush(); - String stackTrace = baos.toString(); - assertThat(stackTrace.contains(SimpleLinkageErrorClass.class.getSimpleName() + ".")).isTrue(); - } - } - } - @Configuration static class BeanMethodConfiguration { @@ -84,33 +60,6 @@ public SimpleFactoryBean simpleFactoryBean(ApplicationContext applicationContext } - @Configuration - static class BeanMethodConfigurationWithError { - - @Bean - public SimpleFactoryBean simpleFactoryBean(ApplicationContext applicationContext) { - SimpleFactoryBean simpleFactoryBean = new SimpleFactoryBean(applicationContext); - // cause a linkage error - new SimpleLinkageErrorClass(); - return simpleFactoryBean; - } - } - - - @Configuration - static class StringTypeConfiguration { - - @Autowired - private String foo; - - @Bean - public String foo() { - return "foo"; - } - - } - - static class SimpleFactoryBean implements FactoryBean { public SimpleFactoryBean(ApplicationContext applicationContext) { @@ -127,17 +76,4 @@ public Class getObjectType() { } } - - static class SimpleLinkageErrorClass { - - private static final int ERROR = checkError(); - - public SimpleLinkageErrorClass() { - } - - private static int checkError() { - throw new NoSuchMethodError(); - } - } - } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.java new file mode 100644 index 000000000000..aa814c2bbdee --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.java @@ -0,0 +1,154 @@ +/* + * Copyright 2002-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation.configuration; + +import java.io.ByteArrayOutputStream; +import java.io.PrintWriter; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +/** + * @author Liu Dongmiao + */ +public class ExceptionInInitializerTests { + + @Test + public void checkXml() { + try (ClassPathXmlApplicationContext ignored = new ClassPathXmlApplicationContext("ExceptionInInitializerTests.xml", getClass())) { + fail("shouldn't happen"); + } catch (BeanCreationException ex) { + checkBeanCreationException(ex); + } + } + + @Test + public void checkAnnotation() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) { + context.register(BeanMethodConfiguration.class); + context.refresh(); + fail("shouldn't happen"); + } catch (BeanCreationException ex) { + checkBeanCreationException(ex); + } + } + + private static void checkBeanCreationException(BeanCreationException ex) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintWriter pw = new PrintWriter(baos); + ex.printStackTrace(pw); + pw.flush(); + String stackTrace = baos.toString(); + assertThat(stackTrace.contains(".")).isTrue(); + assertThat(stackTrace.contains("java.lang.NoClassDefFoundError")).isFalse(); + } + + + @Configuration + static class BeanMethodConfiguration { + + @Bean + public String foo() { + return "foo"; + } + + @Bean + public AutowiredBean autowiredBean() { + return new AutowiredBean(); + } + + @Bean + public SimpleFactoryBean simpleFactoryBean() { + SimpleFactoryBean bean = new SimpleFactoryBean(); + bean.setObjectType(SimpleExceptionInInitializer2.class); + return bean; + } + } + + + static class AutowiredBean { + + protected String foo; + + @Autowired + public void setFoo(String foo) { + this.foo = foo; + } + } + + + static class SimpleBean { + + protected Class objectType; + + protected Object object; + + public void setObjectType(Class objectType) { + this.objectType = objectType; + try { + this.object = objectType.newInstance(); + } catch (ReflectiveOperationException ex) { + throw new RuntimeException(ex); + } + } + } + + + static class SimpleFactoryBean extends SimpleBean implements FactoryBean { + + @Override + public Object getObject() { + return object; + } + + @Override + public Class getObjectType() { + return objectType; + } + } + + + static class SimpleExceptionInInitializer1 { + + private static final int ERROR = callInClinit(); + + private static int callInClinit() { + throw new UnsupportedOperationException(); + } + } + + + static class SimpleExceptionInInitializer2 { + + private static final int ERROR = callInClinit(); + + private static int callInClinit() { + throw new UnsupportedOperationException(); + } + } + +} diff --git a/spring-context/src/test/resources/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.xml b/spring-context/src/test/resources/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.xml new file mode 100644 index 000000000000..b41a26f6e482 --- /dev/null +++ b/spring-context/src/test/resources/org/springframework/context/annotation/configuration/ExceptionInInitializerTests.xml @@ -0,0 +1,11 @@ + + + + + + + + \ No newline at end of file