From 9713bfc7656aca6a43f5ab58b090949c2146e239 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 24 Feb 2021 14:09:47 +0000 Subject: [PATCH] Enable the logging shutdown hook by default This commit updates LoggingApplicationListener to register the logging shutdown hook by default. The hook is detrimental in a war deployment as it may pin parts of an application in memory after it has been undeployed. For this reason, the hook is still disabled by default in war deployments. This is achieved by setting an attribute on the servlet context in SpringBootServletInitializer that is then consumed via the Environment by LoggingApplicationListener. Closes gh-25046 --- .../docs/asciidoc/spring-boot-features.adoc | 15 ++++++------ .../logging/LoggingApplicationListener.java | 4 ++-- .../support/SpringBootServletInitializer.java | 2 ++ ...itional-spring-configuration-metadata.json | 4 ++-- .../LoggingApplicationListenerTests.java | 23 ++++++++++++------- .../SpringBootServletInitializerTests.java | 7 ++++-- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc index 17649eb7b4b0..11a2d9ba2f42 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/spring-boot-features.adoc @@ -1912,20 +1912,19 @@ Spring Boot includes the following pre-defined logging groups that can be used o [[boot-features-log-shutdown-hook]] === Using a Log Shutdown Hook -In order to release logging resources it is usually a good idea to stop the logging system when your application terminates. -Unfortunately, there's no single way to do this that will work with all application types. -If your application has complex context hierarchies or is deployed as a war file, you'll need to investigate the options provided directly by the underlying logging system. +In order to release logging resources when your application terminates, a shutdown hook that will trigger log system cleanup when the JVM exits is provided. +This shutdown hook is registered automatically unless your application is deployed as a war file. +If your application has complex context hierarchies the shutdown hook may not meet your needs. +If it does not, disable the shutdown hook and investigate the options provided directly by the underlying logging system. For example, Logback offers http://logback.qos.ch/manual/loggingSeparation.html[context selectors] which allow each Logger to be created in its own context. - -For simple "single jar" applications deployed in their own JVM, you can use the `logging.register-shutdown-hook` property. -Setting `logging.register-shutdown-hook` to `true` will register a shutdown hook that will trigger log system cleanup when the JVM exits. - +You can use the configprop:logging.register-shutdown-hook[] property to disable the shutdown hook. +Setting it to `false` will disable the registration. You can set the property in your `application.properties` or `application.yaml` file: [source,yaml,indent=0,configprops,configblocks] ---- logging: - register-shutdown-hook: true + register-shutdown-hook: false ---- diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java index c22c7ba127de..b411a814e2f7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -398,7 +398,7 @@ private BiConsumer getLogLevelConfigurer(LoggingSystem system) } private void registerShutdownHookIfNecessary(Environment environment, LoggingSystem loggingSystem) { - boolean registerShutdownHook = environment.getProperty(REGISTER_SHUTDOWN_HOOK_PROPERTY, Boolean.class, false); + boolean registerShutdownHook = environment.getProperty(REGISTER_SHUTDOWN_HOOK_PROPERTY, Boolean.class, true); if (registerShutdownHook) { Runnable shutdownHandler = loggingSystem.getShutdownHandler(); if (shutdownHandler != null && shutdownHookRegistered.compareAndSet(false, true)) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializer.java index 38c4b7ccaa32..255598344ec2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializer.java @@ -34,6 +34,7 @@ import org.springframework.boot.builder.ParentContextApplicationContextInitializer; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; +import org.springframework.boot.context.logging.LoggingApplicationListener; import org.springframework.boot.web.servlet.ServletContextInitializer; import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.springframework.context.ApplicationContext; @@ -89,6 +90,7 @@ protected final void setRegisterErrorPageFilter(boolean registerErrorPageFilter) @Override public void onStartup(ServletContext servletContext) throws ServletException { + servletContext.setAttribute(LoggingApplicationListener.REGISTER_SHUTDOWN_HOOK_PROPERTY, false); // Logger initialization is deferred in case an ordered // LogServletContextInitializer is being used this.logger = LogFactory.getLog(getClass()); diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 99ecbf0fe077..4a1219c97c21 100644 --- a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -109,9 +109,9 @@ { "name": "logging.register-shutdown-hook", "type": "java.lang.Boolean", - "description": "Register a shutdown hook for the logging system when it is initialized.", + "description": "Register a shutdown hook for the logging system when it is initialized. Disabled automatically when deployed as a war file.", "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener", - "defaultValue": false + "defaultValue": true }, { "name": "logging.pattern.rolling-file-name", diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java index 88a9d5f2f273..8ea44001975f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/logging/LoggingApplicationListenerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-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. @@ -27,6 +27,7 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Handler; import java.util.logging.LogManager; import java.util.logging.Logger; @@ -405,24 +406,30 @@ void overrideExceptionConversionWord() { } @Test - void shutdownHookIsNotRegisteredByDefault() { + void shutdownHookIsRegisteredByDefault() throws Exception { TestLoggingApplicationListener listener = new TestLoggingApplicationListener(); + Object registered = ReflectionTestUtils.getField(listener, TestLoggingApplicationListener.class, + "shutdownHookRegistered"); + ((AtomicBoolean) registered).set(false); System.setProperty(LoggingSystem.class.getName(), TestShutdownHandlerLoggingSystem.class.getName()); multicastEvent(listener, new ApplicationStartingEvent(this.bootstrapContext, new SpringApplication(), NO_ARGS)); listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); - assertThat(listener.shutdownHook).isNull(); + assertThat(listener.shutdownHook).isNotNull(); + listener.shutdownHook.start(); + assertThat(TestShutdownHandlerLoggingSystem.shutdownLatch.await(30, TimeUnit.SECONDS)).isTrue(); } @Test - void shutdownHookCanBeRegistered() throws Exception { + void shutdownHookRegistrationCanBeDisabled() throws Exception { TestLoggingApplicationListener listener = new TestLoggingApplicationListener(); + Object registered = ReflectionTestUtils.getField(listener, TestLoggingApplicationListener.class, + "shutdownHookRegistered"); + ((AtomicBoolean) registered).set(false); System.setProperty(LoggingSystem.class.getName(), TestShutdownHandlerLoggingSystem.class.getName()); - addPropertiesToEnvironment(this.context, "logging.register_shutdown_hook=true"); + addPropertiesToEnvironment(this.context, "logging.register_shutdown_hook=false"); multicastEvent(listener, new ApplicationStartingEvent(this.bootstrapContext, new SpringApplication(), NO_ARGS)); listener.initialize(this.context.getEnvironment(), this.context.getClassLoader()); - assertThat(listener.shutdownHook).isNotNull(); - listener.shutdownHook.start(); - assertThat(TestShutdownHandlerLoggingSystem.shutdownLatch.await(30, TimeUnit.SECONDS)).isTrue(); + assertThat(listener.shutdownHook).isNull(); } @Test diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java index 8aaf6e25aedd..c6d428973eb7 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java @@ -36,6 +36,7 @@ import org.springframework.boot.SpringApplication; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; +import org.springframework.boot.context.logging.LoggingApplicationListener; import org.springframework.boot.testsupport.system.CapturedOutput; import org.springframework.boot.testsupport.system.OutputCaptureExtension; import org.springframework.boot.web.embedded.undertow.UndertowServletWebServerFactory; @@ -113,8 +114,10 @@ void mainClassHasSensibleDefault() { } @Test - void shutdownHookIsNotRegistered() { - new WithConfigurationAnnotation().createRootApplicationContext(this.servletContext); + void shutdownHooksAreNotRegistered() throws ServletException { + new WithConfigurationAnnotation().onStartup(this.servletContext); + assertThat(this.servletContext.getAttribute(LoggingApplicationListener.REGISTER_SHUTDOWN_HOOK_PROPERTY)) + .isEqualTo(false); assertThat(this.application).hasFieldOrPropertyWithValue("registerShutdownHook", false); }