From e1522df8a547fbfc3a35bc959dbb8747aeb35ae7 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 29 Mar 2023 20:59:23 +1300 Subject: [PATCH] Fix for #3012 Manual server shutdown leads to memory leak The issue being that shutdown() // with no args was not calling ShutdownManager.unregisterDatabase(this) noting that shutdown(boolean, boolean) did. This change merges the old shutdownInternal(boolean, boolean) method into shutdown(boolean, boolean) and simplifies shutdown() to just call shutdown(boolean, boolean). --- .../java/io/ebean/event/ShutdownManager.java | 2 +- .../server/core/DefaultServer.java | 49 +++++++------------ .../ModelBuild_explicitSequencesTest.java | 4 +- 3 files changed, 20 insertions(+), 35 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java b/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java index 91c351f30..458199c8e 100644 --- a/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java +++ b/ebean-api/src/main/java/io/ebean/event/ShutdownManager.java @@ -143,7 +143,7 @@ public final class ShutdownManager { } // shutdown any registered servers that have not // already been shutdown manually - for (Database server : databases) { + for (Database server : new ArrayList<>(databases)) { try { server.shutdown(); } catch (Exception ex) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index c8bd2d3d1..98356f418 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -354,50 +354,35 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { @Override public void shutdown() { - lock.lock(); - try { - shutdownInternal(true, false); - } finally { - lock.unlock(); - } + shutdown(true, false); } - /** - * Shutting down manually. - */ @Override public void shutdown(boolean shutdownDataSource, boolean deregisterDriver) { lock.lock(); try { ShutdownManager.unregisterDatabase(this); - shutdownInternal(shutdownDataSource, deregisterDriver); + log.log(TRACE, "shutting down instance {0}", serverName); + if (shutdown) { + // already shutdown + return; + } + shutdownPlugins(); + autoTuneService.shutdown(); + // shutdown background threads + backgroundExecutor.shutdown(); + // shutdown DataSource (if its an Ebean one) + transactionManager.shutdown(shutdownDataSource, deregisterDriver); + dumpMetrics(); + shutdown = true; + if (shutdownDataSource) { + config.setDataSource(null); + } } finally { lock.unlock(); } } - /** - * Shutdown the services like threads and DataSource. - */ - private void shutdownInternal(boolean shutdownDataSource, boolean deregisterDriver) { - log.log(TRACE, "shutting down instance {0}", serverName); - if (shutdown) { - // already shutdown - return; - } - shutdownPlugins(); - autoTuneService.shutdown(); - // shutdown background threads - backgroundExecutor.shutdown(); - // shutdown DataSource (if its an Ebean one) - transactionManager.shutdown(shutdownDataSource, deregisterDriver); - dumpMetrics(); - shutdown = true; - if (shutdownDataSource) { - config.setDataSource(null); - } - } - private void dumpMetrics() { if (config.isDumpMetricsOnShutdown()) { new DumpMetrics(this, config.getDumpMetricsOptions()).dump(); diff --git a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuild_explicitSequencesTest.java b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuild_explicitSequencesTest.java index 8a678aae3..77f78452e 100644 --- a/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuild_explicitSequencesTest.java +++ b/ebean-ddl-generator/src/test/java/io/ebeaninternal/dbmigration/model/build/ModelBuild_explicitSequencesTest.java @@ -1,13 +1,13 @@ package io.ebeaninternal.dbmigration.model.build; -import io.localtest.BaseTestCase; import io.ebean.DatabaseFactory; import io.ebean.config.DatabaseConfig; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.dbmigration.ddlgeneration.DdlOptions; import io.ebeaninternal.dbmigration.ddlgeneration.Helper; import io.ebeaninternal.dbmigration.model.CurrentModel; +import io.localtest.BaseTestCase; import org.junit.jupiter.api.Test; import org.tests.model.basic.Person; import org.tests.model.basic.Phone; @@ -66,7 +66,7 @@ class ModelBuild_explicitSequencesTest extends BaseTestCase { .startsWith("-- Generated by ebean") .endsWith(Helper.asText(this, "/assert/ModelBuild_explicitSequencesTest/pg-apply.sql")); } finally { - ebeanServer.shutdown(); + ebeanServer.shutdown(true, false); } }