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

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
 ----
 
 

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 @@ public class LoggingApplicationListener implements GenericApplicationListener {
 	}
 
 	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)) {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializer.java

@@ -34,6 +34,7 @@ import org.springframework.boot.SpringApplication;
 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 @@ public abstract class SpringBootServletInitializer implements WebApplicationInit
 
 	@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());

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",

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.Map;
 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 @@ class LoggingApplicationListenerTests {
 	}
 
 	@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

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/support/SpringBootServletInitializerTests.java

@@ -36,6 +36,7 @@ import org.mockito.ArgumentCaptor;
 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 @@ class SpringBootServletInitializerTests {
 	}
 
 	@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);
 	}