Skip to content

Custom LoggingSystemShutdownHook to initiate/wait for applicationContext.close() #4755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

brenuart
Copy link
Contributor

LoggingApplicationListener registers its own shutdown hook (if so configured) and delegates to the handler provided by the LoggingSystem. The registered hook closes the application context before stopping the logging system.

The implementation should ideally consult the SpringApplication to detect if it is configured to register its own hook or not. Unfortunately this information is not available.

Maybe a cleaner solution is for the SpringApplication to allow for registration of additional hooks that would be invoked after its own. Something like a sequence of hooks, possibly ordered, where the SpringApplication would register its own with higher precedence if so configured. That composite hook would always be registered.

See #4681

…ation context shutdown before proceeding with the actual stop of the logging system.
@brenuart brenuart changed the title Custom LoggingSystemShutdownHook to initiate/wait for applicationContext.close() gh4681: Custom LoggingSystemShutdownHook to initiate/wait for applicationContext.close() Dec 11, 2015
@brenuart brenuart changed the title gh4681: Custom LoggingSystemShutdownHook to initiate/wait for applicationContext.close() Custom LoggingSystemShutdownHook to initiate/wait for applicationContext.close() Dec 11, 2015
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 11, 2015
@wilkinsona
Copy link
Member

@brenuart The Travis build failed for this change. Could you please address the formatting and javadoc problems reported by checkstyle:

/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:124: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:222: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:225:68: Expected @param tag for 'event'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:227: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:231: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:370:96: ')' is preceded with whitespace.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:399: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:400: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:402: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:404: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:405:17: Redundant 'public' modifier.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:408: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:412: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:415: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:427:27: 'if' is not followed by whitespace.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:427:28: '(' is followed by whitespace.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:427:29: Reference to instance variable 'applicationContext' needs "this.".
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:427:55: ')' is preceded with whitespace.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:428:33: Reference to instance variable 'applicationContext' needs "this.".
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:430: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:431: Line matches the illegal pattern 'Trailing whitespace'.
/Users/awilkinson/dev/spring/spring-boot/spring-boot/src/main/java/org/springframework/boot/logging/LoggingApplicationListener.java:434:25: Reference to instance variable 'loggingSystemShutdownHandler' needs "this.".

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 14, 2015
@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 16, 2016
@philwebb philwebb added type: blocker An issue that is blocking us from releasing and removed type: blocker An issue that is blocking us from releasing for: team-attention An issue we'd like other members of the team to review labels Apr 6, 2016
@wilkinsona
Copy link
Member

I think we're in danger of building a house of cards if we pursue the shutdown-hook-based approach. As I said in my comment on 4681, the logging system is now only cleaned up as a result of the root context being closed. This is our preferred approach for shutting down the logging system and should be used in preference to a logging system shutdown hook.

@wilkinsona wilkinsona closed this Apr 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants