From b7a8b0f19b9cf0eb09ab24d4014766695e08ecc7 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 20 Oct 2020 08:48:27 +0200 Subject: [PATCH 1/2] Hacking See gh-23740 --- .../HibernateMetricsAutoConfiguration.java | 17 +++++++++++++++- ...ibernateMetricsAutoConfigurationTests.java | 20 ++++++++++++++++++- .../src/test/resources/city-data.sql | 1 + .../src/test/resources/city-schema.sql | 7 +++++++ .../build.gradle | 1 + .../src/main/resources/application.properties | 2 +- 6 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-data.sql create mode 100644 spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-schema.sql diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java index fa111f2b21c..d4cac44a49d 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java @@ -26,6 +26,7 @@ import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.binder.jpa.HibernateMetrics; import org.hibernate.SessionFactory; +import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration; import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration; @@ -50,11 +51,25 @@ import org.springframework.util.StringUtils; SimpleMetricsExportAutoConfiguration.class }) @ConditionalOnClass({ EntityManagerFactory.class, SessionFactory.class, MeterRegistry.class }) @ConditionalOnBean({ EntityManagerFactory.class, MeterRegistry.class }) -public class HibernateMetricsAutoConfiguration { +public class HibernateMetricsAutoConfiguration implements SmartInitializingSingleton { private static final String ENTITY_MANAGER_FACTORY_SUFFIX = "entityManagerFactory"; + private Map entityManagerFactories; + + private MeterRegistry meterRegistry; + @Autowired + void injectDependencies(Map entityManagerFactories, MeterRegistry meterRegistry) { + this.entityManagerFactories = entityManagerFactories; + this.meterRegistry = meterRegistry; + } + + @Override + public void afterSingletonsInstantiated() { + bindEntityManagerFactoriesToRegistry(this.entityManagerFactories, this.meterRegistry); + } + public void bindEntityManagerFactoriesToRegistry(Map entityManagerFactories, MeterRegistry registry) { entityManagerFactories.forEach((name, factory) -> bindEntityManagerFactoryToRegistry(name, factory, registry)); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java index 12bbb0e57d3..f85f0b0bf43 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java @@ -35,6 +35,7 @@ import org.mockito.ArgumentMatchers; import org.springframework.boot.actuate.autoconfigure.metrics.test.MetricsRun; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; +import org.springframework.boot.autoconfigure.orm.jpa.EntityManagerFactoryBuilderCustomizer; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.orm.jpa.EntityManagerFactoryBuilder; import org.springframework.boot.test.context.FilteredClassLoader; @@ -42,6 +43,8 @@ import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; +import org.springframework.core.task.SimpleAsyncTaskExecutor; +import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; @@ -58,7 +61,7 @@ import static org.mockito.Mockito.mock; */ class HibernateMetricsAutoConfigurationTests { - private ApplicationContextRunner contextRunner = new ApplicationContextRunner().with(MetricsRun.simple()) + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner().with(MetricsRun.simple()) .withConfiguration(AutoConfigurations.of(DataSourceAutoConfiguration.class, HibernateJpaAutoConfiguration.class, HibernateMetricsAutoConfiguration.class)) .withUserConfiguration(BaseConfiguration.class); @@ -127,6 +130,21 @@ class HibernateMetricsAutoConfigurationTests { }); } + @Test + void entityManagerFactoryInstrumentationDoesNotDeadlockWithDeferredInitialization() { + this.contextRunner + .withPropertyValues("spring.jpa.properties.hibernate.generate_statistics:true", + "spring.datasource.schema=city-schema.sql", "spring.datasource.data=city-data.sql") + .withBean(EntityManagerFactoryBuilderCustomizer.class, + () -> (builder) -> builder.setBootstrapExecutor(new SimpleAsyncTaskExecutor())) + .run((context) -> { + JdbcTemplate jdbcTemplate = new JdbcTemplate(context.getBean(DataSource.class)); + assertThat(jdbcTemplate.queryForObject("SELECT COUNT(*) from CITY", Integer.class)).isEqualTo(1); + MeterRegistry registry = context.getBean(MeterRegistry.class); + registry.get("hibernate.statements").tags("entityManagerFactory", "entityManagerFactory").meter(); + }); + } + @Configuration(proxyBeanMethods = false) static class BaseConfiguration { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-data.sql b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-data.sql new file mode 100644 index 00000000000..eb08623b4cf --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-data.sql @@ -0,0 +1 @@ +INSERT INTO CITY (ID, NAME, STATE, COUNTRY, MAP) values (2000, 'Washington', 'DC', 'US', 'Google'); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-schema.sql b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-schema.sql new file mode 100644 index 00000000000..7e3a3462e00 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/resources/city-schema.sql @@ -0,0 +1,7 @@ +CREATE TABLE CITY ( + id INTEGER IDENTITY PRIMARY KEY, + name VARCHAR(30), + state VARCHAR(30), + country VARCHAR(30), + map VARCHAR(30) +); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/build.gradle b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/build.gradle index c54a169a6f1..d009a1eeb62 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/build.gradle +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/build.gradle @@ -8,6 +8,7 @@ description = "Spring Boot Data JPA smoke test" dependencies { implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-data-jpa")) implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-web")) + implementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-actuator")) runtimeOnly("com.h2database:h2") diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/src/main/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/src/main/resources/application.properties index 265a5e9a851..a782f2e74dd 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/src/main/resources/application.properties +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-data-jpa/src/main/resources/application.properties @@ -1,4 +1,4 @@ spring.h2.console.enabled=true spring.jpa.open-in-view=true -spring.data.jpa.repositories.bootstrap-mode=default +spring.data.jpa.repositories.bootstrap-mode=deferred logging.level.org.hibernate.SQL=debug From f4e822f650cc0e94183a8210d89df572b2c4c532 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 20 Oct 2020 09:38:24 +0200 Subject: [PATCH 2/2] Prevent access to the EMF within the singleton lock This commit makes sure to defer registration of hibernate statistics outside of the singleton lock as it can lead to deadlocks when the EntityManagerFactory is initialized in deferred mode. Closes gh-23740 --- .../metrics/orm/jpa/HibernateMetricsAutoConfiguration.java | 2 +- .../metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java index d4cac44a49d..998aaf7f4ea 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java index f85f0b0bf43..5c4c2962fb2 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/orm/jpa/HibernateMetricsAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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.