Use a shutdown task to clean up ORM resources

marko-bekhta · marko-bekhta · commit 44c2ccbd3aac · 2025-06-17T21:06:56.000+02:00
diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java
@@ -14,9 +14,7 @@
 import java.util.stream.Collectors;
 
 import jakarta.enterprise.context.ApplicationScoped;
-import jakarta.enterprise.inject.Any;
 import jakarta.enterprise.inject.Default;
-import jakarta.enterprise.inject.Instance;
 import jakarta.inject.Singleton;
 import jakarta.persistence.AttributeConverter;
 import jakarta.transaction.TransactionManager;
@@ -37,11 +35,9 @@
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.MethodInfo;
-import org.jboss.jandex.ParameterizedType;
 import org.jboss.jandex.Type;
 import org.objectweb.asm.ClassVisitor;
 
-import io.agroal.api.AgroalDataSource;
 import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
 import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
 import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
@@ -168,32 +164,15 @@ void generateJpaConfigBean(HibernateOrmRecorder recorder,
                 .scope(Singleton.class)
                 .unremovable()
                 .setRuntimeInit()
-                .supplier(recorder.jpaConfigSupplier(hibernateOrmRuntimeConfig))
-                .destroyer(JPAConfig.Destroyer.class);
-
-        // Add a synthetic dependency from JPAConfig to any datasource/pool,
-        // so that JPAConfig is destroyed before the datasource/pool.
-        // The alternative would be adding an application destruction observer
-        // (@Observes @BeforeDestroyed(ApplicationScoped.class)) to JPAConfig,
-        // but that would force initialization of JPAConfig upon application shutdown,
-        // which may cause cascading failures if the shutdown happened before JPAConfig was initialized.
-        if (capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
-            configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
-                    new Type[] { ClassType.create(DotName.createSimple("io.vertx.sqlclient.Pool")) }, null),
-                    AnnotationInstance.builder(Any.class).build());
-        } else {
-            configurator.addInjectionPoint(ParameterizedType.create(DotName.createSimple(Instance.class),
-                    new Type[] { ClassType.create(DotName.createSimple(AgroalDataSource.class)) }, null),
-                    AnnotationInstance.builder(Any.class).build());
-        }
+                .supplier(recorder.jpaConfigSupplier(hibernateOrmRuntimeConfig));
 
         syntheticBeanBuildItemBuildProducer.produce(configurator.done());
     }
 
     // These beans must be initialized at runtime because their initialization
     // depends on runtime configuration (to activate/deactivate a persistence unit)
-    @Record(ExecutionTime.RUNTIME_INIT)
     @BuildStep
+    @Record(ExecutionTime.RUNTIME_INIT)
     void generateDataSourceBeans(HibernateOrmRecorder recorder,
             List<PersistenceUnitDescriptorBuildItem> persistenceUnitDescriptors,
             ImpliedBlockingPersistenceUnitTypeBuildItem impliedBlockingPersistenceUnitType,
diff --git a/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java b/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java
@@ -93,6 +93,7 @@
 import io.quarkus.deployment.builditem.LogCategoryBuildItem;
 import io.quarkus.deployment.builditem.NativeImageFeatureBuildItem;
 import io.quarkus.deployment.builditem.ServiceStartBuildItem;
+import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
 import io.quarkus.deployment.builditem.SystemPropertyBuildItem;
 import io.quarkus.deployment.builditem.TransformedClassesBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.NativeImageProxyDefinitionBuildItem;
@@ -674,12 +675,13 @@ public PersistenceProviderSetUpBuildItem setupPersistenceProvider(HibernateOrmRe
     @Consume(SyntheticBeansRuntimeInitBuildItem.class)
     @Record(RUNTIME_INIT)
     public ServiceStartBuildItem startPersistenceUnits(HibernateOrmRecorder recorder, BeanContainerBuildItem beanContainer,
+            ShutdownContextBuildItem shutdownContextBuildItem,
             List<JdbcDataSourceBuildItem> dataSourcesConfigured,
             JpaModelBuildItem jpaModel,
             List<JdbcDataSourceSchemaReadyBuildItem> schemaReadyBuildItem,
             List<PersistenceProviderSetUpBuildItem> persistenceProviderSetUp) throws Exception {
         if (hasEntities(jpaModel)) {
-            recorder.startAllPersistenceUnits(beanContainer.getValue());
+            recorder.startAllPersistenceUnits(beanContainer.getValue(), shutdownContextBuildItem);
         }
 
         return new ServiceStartBuildItem("Hibernate ORM");
diff --git a/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java b/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/HibernateOrmRecorder.java
@@ -33,6 +33,7 @@
 import io.quarkus.hibernate.orm.runtime.proxies.PreGeneratedProxies;
 import io.quarkus.hibernate.orm.runtime.schema.SchemaManagementIntegrator;
 import io.quarkus.hibernate.orm.runtime.tenant.DataSourceTenantConnectionResolver;
+import io.quarkus.runtime.ShutdownContext;
 import io.quarkus.runtime.annotations.Recorder;
 
 /**
@@ -100,8 +101,16 @@ public Supplier<JPAConfig> jpaConfigSupplier(HibernateOrmRuntimeConfig config) {
         return () -> new JPAConfig(config);
     }
 
-    public void startAllPersistenceUnits(BeanContainer beanContainer) {
-        beanContainer.beanInstance(JPAConfig.class).startAll();
+    public void startAllPersistenceUnits(BeanContainer beanContainer, ShutdownContext shutdownContext) {
+        JPAConfig jpaConfig = beanContainer.beanInstance(JPAConfig.class);
+        // NOTE:
+        //  - We register the shutdown task before we start any PUs,
+        //    This way we'll be able to clean up even if one of the PUs fails to start while others already did.
+        //  - The step that starts the PUs returns the ServiceStartBuildItem, this in turn ensures that
+        //    the shutdown task that triggers the ShutdownEvent will be registered after this one,
+        //    and users will have access to the "ORM stuff" in their listeners.
+        shutdownContext.addShutdownTask(jpaConfig::shutdown);
+        jpaConfig.startAll();
     }
 
     public Function<SyntheticCreationalContext<SessionFactory>, SessionFactory> sessionFactorySupplier(
diff --git a/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/JPAConfig.java b/extensions/hibernate-orm/runtime/src/main/java/io/quarkus/hibernate/orm/runtime/JPAConfig.java
@@ -10,14 +10,12 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import jakarta.enterprise.context.spi.CreationalContext;
 import jakarta.inject.Inject;
 import jakarta.persistence.EntityManagerFactory;
 import jakarta.persistence.Persistence;
 
 import org.jboss.logging.Logger;
 
-import io.quarkus.arc.BeanDestroyer;
 import io.quarkus.hibernate.orm.runtime.boot.QuarkusPersistenceUnitDescriptor;
 
 public class JPAConfig {
@@ -129,18 +127,22 @@ public boolean getRequestScopedSessionEnabled() {
         return this.requestScopedSessionEnabled;
     }
 
-    public static class Destroyer implements BeanDestroyer<JPAConfig> {
-        @Override
-        public void destroy(JPAConfig instance, CreationalContext<JPAConfig> creationalContext, Map<String, Object> params) {
-            for (LazyPersistenceUnit factory : instance.persistenceUnits.values()) {
+    void shutdown() {
+        LOGGER.trace("Starting to shut down Hibernate ORM persistence units.");
+        for (LazyPersistenceUnit factory : this.persistenceUnits.values()) {
+            if (factory.isStarted()) {
                 try {
+                    LOGGER.tracef("Closing Hibernate ORM persistence unit: %s.", factory.name);
                     factory.close();
                 } catch (Exception e) {
                     LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);
                 }
+            } else {
+                LOGGER.tracef("Skipping Hibernate ORM persistence unit, that failed to start: %s.", factory.name);
             }
-            instance.persistenceUnits.clear();
         }
+        this.persistenceUnits.clear();
+        LOGGER.trace("Finished shutting down Hibernate ORM persistence units.");
     }
 
     static final class LazyPersistenceUnit {
@@ -175,6 +177,10 @@ public synchronized void close() {
                 emf.close();
             }
         }
+
+        boolean isStarted() {
+            return !closed && value != null;
+        }
     }
 
 }
diff --git a/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/pom.xml b/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/pom.xml
@@ -50,6 +50,11 @@
             <artifactId>quarkus-junit5</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-junit5-internal</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-test-h2</artifactId>
@@ -171,6 +176,32 @@
                         <configuration>
                             <skip>false</skip>
                         </configuration>
+                        <executions>
+                            <execution>
+                                <id>default-test</id>
+                                <goals>
+                                    <goal>test</goal>
+                                </goals>
+                                <configuration>
+                                    <!-- These tests seem to leak metaspace memory
+                                         so we run them in a separate, short-lived JVM -->
+                                    <excludedGroups>devmode</excludedGroups>
+                                </configuration>
+                            </execution>
+                            <execution>
+                                <id>devmode-test</id>
+                                <goals>
+                                    <goal>test</goal>
+                                </goals>
+                                <configuration>
+                                    <!-- These tests seem to leak metaspace memory
+                                         so we run them in a separate, short-lived JVM -->
+                                    <groups>devmode</groups>
+                                    <!-- We could consider setting reuseForks=false if we
+                                         really need to run every single test in its own JVM -->
+                                </configuration>
+                            </execution>
+                        </executions>
                     </plugin>
                     <plugin>
                         <artifactId>maven-failsafe-plugin</artifactId>
diff --git a/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeFailingOrmTest.java b/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeFailingOrmTest.java
@@ -0,0 +1,80 @@
+package io.quarkus.it.hibernate.search.orm.elasticsearch.coordination.outboxpolling;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusDevModeTest;
+import io.restassured.RestAssured;
+
+@Tag("devmode")
+public class HibernateSearchDevModeFailingOrmTest {
+
+    @RegisterExtension
+    static final QuarkusDevModeTest config = new QuarkusDevModeTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(HibernateSearchOutboxPollingTestResource.class, Person.class, OutboxPollingTestUtils.class)
+                    .addAsResource("application-dev-mode.properties", "application.properties"))
+            .setLogRecordPredicate(r -> true);
+
+    static String APPLICATION_PROPERTIES;
+
+    @BeforeAll
+    static void beforeAll() throws Exception {
+        APPLICATION_PROPERTIES = Files
+                .readString(
+                        Paths.get(HibernateSearchDevModeFailingOrmTest.class.getResource("/application-dev-mode.properties")
+                                .toURI()));
+    }
+
+    @Test
+    public void smoke() {
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(200);
+
+        // now add a property that will fail the datasource:
+        config.modifyResourceFile(
+                "application.properties",
+                s -> APPLICATION_PROPERTIES + """
+                        quarkus.datasource.jdbc.min-size=20
+                        """);
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(500);
+
+        // add any change to get the shutdown of a failed app completed:
+        config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);
+
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(200);
+
+        // At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
+        // Hence we expect the following things logged:
+        // initial run:
+        //  - profile activated (after a successful startup)
+        //  - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
+        // first reload:
+        //  - ORM message telling us that the PU closing won't happen as the PU failed to start
+        // second reload:
+        //  - profile activated (after a successful startup)
+        //  - no ORM shutdown message, as that will happen after the test body finishes.
+        assertThat(config.getLogRecords()).satisfiesOnlyOnce(
+                r -> {
+                    assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
+                    assertThat(r.getParameters()).containsExactly("<default>");
+                });
+        assertThat(config.getLogRecords()).satisfiesOnlyOnce(
+                r -> {
+                    assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
+                    assertThat(r.getParameters()).containsExactly("<default>");
+                });
+        assertThat(config.getLogRecords().stream()
+                .filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
+                .hasSize(2);
+    }
+}
diff --git a/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeFailingSearchTest.java b/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeFailingSearchTest.java
@@ -0,0 +1,80 @@
+package io.quarkus.it.hibernate.search.orm.elasticsearch.coordination.outboxpolling;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusDevModeTest;
+import io.restassured.RestAssured;
+
+@Tag("devmode")
+public class HibernateSearchDevModeFailingSearchTest {
+
+    @RegisterExtension
+    static final QuarkusDevModeTest config = new QuarkusDevModeTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(HibernateSearchOutboxPollingTestResource.class, Person.class, OutboxPollingTestUtils.class)
+                    .addAsResource("application-dev-mode.properties", "application.properties"))
+            .setLogRecordPredicate(r -> true);
+
+    static String APPLICATION_PROPERTIES;
+
+    @BeforeAll
+    static void beforeAll() throws Exception {
+        APPLICATION_PROPERTIES = Files
+                .readString(Paths
+                        .get(HibernateSearchDevModeFailingSearchTest.class.getResource("/application-dev-mode.properties")
+                                .toURI()));
+    }
+
+    @Test
+    public void smoke() {
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(200);
+
+        // now add a property that will fail the search, but since search is started through ORM integrator.. we are actually failing ORM startup:
+        config.modifyResourceFile(
+                "application.properties",
+                s -> APPLICATION_PROPERTIES.replace(
+                        "quarkus.hibernate-search-orm.elasticsearch.hosts=${elasticsearch.hosts:localhost:9200}",
+                        "quarkus.hibernate-search-orm.elasticsearch.hosts=not-a-localhost:9211"));
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(500);
+
+        // and any change to get the shutdown of a failed app completed:
+        config.modifyResourceFile("application.properties", s -> APPLICATION_PROPERTIES);
+
+        RestAssured.when().put("/test/hibernate-search-outbox-polling/check-agents-running").then()
+                .statusCode(200);
+
+        // At this point we've tried starting the app 3 times: one initial, one failing, one successful live-reloads
+        // Hence we expect the following things logged:
+        // initial run:
+        //  - profile activated (after a successful startup)
+        //  - ORM message after a successful shutdown caused by a following live-reload (closing a PU)
+        // first reload:
+        //  - ORM message telling us that the PU closing won't happen as the PU failed to start
+        // second reload:
+        //  - profile activated (after a successful startup)
+        //  - no ORM shutdown message, as that will happen after the test body finishes.
+        assertThat(config.getLogRecords()).satisfiesOnlyOnce(
+                r -> {
+                    assertThat(r.getMessage()).contains("Closing Hibernate ORM persistence unit");
+                    assertThat(r.getParameters()).containsExactly("<default>");
+                });
+        assertThat(config.getLogRecords()).satisfiesOnlyOnce(
+                r -> {
+                    assertThat(r.getMessage()).contains("Skipping Hibernate ORM persistence unit, that failed to start");
+                    assertThat(r.getParameters()).containsExactly("<default>");
+                });
+        assertThat(config.getLogRecords().stream()
+                .filter(r -> r.getMessage().contains("Profile%s %s activated. %s")))
+                .hasSize(2);
+    }
+}
diff --git a/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeTest.java b/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/java/io/quarkus/it/hibernate/search/orm/elasticsearch/coordination/outboxpolling/HibernateSearchDevModeTest.java
diff --git a/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/resources/application-dev-mode.properties b/integration-tests/hibernate-search-orm-elasticsearch-outbox-polling/src/test/resources/application-dev-mode.properties

Original file line number	Diff line number	Diff line change
`@@ -10,14 +10,12 @@`
`10`	`10`	`import java.util.concurrent.CompletableFuture;`
`11`	`11`	`import java.util.concurrent.ExecutionException;`
`12`	`12`
`13`		`-import jakarta.enterprise.context.spi.CreationalContext;`
`14`	`13`	`import jakarta.inject.Inject;`
`15`	`14`	`import jakarta.persistence.EntityManagerFactory;`
`16`	`15`	`import jakarta.persistence.Persistence;`
`17`	`16`
`18`	`17`	`import org.jboss.logging.Logger;`
`19`	`18`
`20`		`-import io.quarkus.arc.BeanDestroyer;`
`21`	`19`	`import io.quarkus.hibernate.orm.runtime.boot.QuarkusPersistenceUnitDescriptor;`
`22`	`20`
`23`	`21`	`public class JPAConfig {`
`@@ -129,18 +127,22 @@ public boolean getRequestScopedSessionEnabled() {`
`129`	`127`	`return this.requestScopedSessionEnabled;`
`130`	`128`	`}`
`131`	`129`
`132`		`- public static class Destroyer implements BeanDestroyer<JPAConfig> {`
`133`		`- @Override`
`134`		`- public void destroy(JPAConfig instance, CreationalContext<JPAConfig> creationalContext, Map<String, Object> params) {`
`135`		`- for (LazyPersistenceUnit factory : instance.persistenceUnits.values()) {`
	`130`	`+ void shutdown() {`
	`131`	`+ LOGGER.trace("Starting to shut down Hibernate ORM persistence units.");`
	`132`	`+ for (LazyPersistenceUnit factory : this.persistenceUnits.values()) {`
	`133`	`+ if (factory.isStarted()) {`
`136`	`134`	`try {`
	`135`	`+ LOGGER.tracef("Closing Hibernate ORM persistence unit: %s.", factory.name);`
`137`	`136`	`factory.close();`
`138`	`137`	`} catch (Exception e) {`
`139`	`138`	`LOGGER.warn("Unable to close the EntityManagerFactory: " + factory, e);`
`140`	`139`	`}`
	`140`	`+ } else {`
	`141`	`+ LOGGER.tracef("Skipping Hibernate ORM persistence unit, that failed to start: %s.", factory.name);`
`141`	`142`	`}`
`142`		`- instance.persistenceUnits.clear();`
`143`	`143`	`}`
	`144`	`+ this.persistenceUnits.clear();`
	`145`	`+ LOGGER.trace("Finished shutting down Hibernate ORM persistence units.");`
`144`	`146`	`}`
`145`	`147`
`146`	`148`	`static final class LazyPersistenceUnit {`
`@@ -175,6 +177,10 @@ public synchronized void close() {`
`175`	`177`	`emf.close();`
`176`	`178`	`}`
`177`	`179`	`}`
	`180`	`+`
	`181`	`+ boolean isStarted() {`
	`182`	`+ return !closed && value != null;`
	`183`	`+ }`
`178`	`184`	`}`
`179`	`185`
`180`	`186`	`}`